Hi Tom,
On Tue, 15 Oct 2024 at 23:47, Tom Rini <tr...@konsulko.com> wrote:
> Hey all, here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-ad...@coverity.com>
> Date: Tue, Oct 15, 2024 at 5:06 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.r...@gmail.com>
>
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to Das
> U-Boot found with Coverity Scan.
>
> 22 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 20 of 22 defect(s)
>
> [snip]
>
> *** CID 510809: Resource leaks (RESOURCE_LEAK)
> /lib/mbedtls/pkcs7_parser.c: 385 in x509_populate_sinfo()
> 379 signed_info);
> 380 if (ret)
> 381 goto out_err_sinfo;
> 382
> 383 no_authattrs:
> 384 *sinfo = signed_info;
> >>> CID 510809: Resource leaks (RESOURCE_LEAK)
> >>> Variable "mctx" going out of scope leaks the storage it points to.
> 385 return 0;
> 386
> 387 out_err_sinfo:
> 388 pkcs7_free_sinfo_mbedtls_ctx(mctx);
> 389 out_no_mctx:
> 390 public_key_signature_free(s);
>
> I will post a patch to fix this defect .
[snip]
> *** CID 510807: Control flow issues (DEADCODE)
> /lib/mbedtls/external/mbedtls/library/x509_crt.c: 2750 in
> x509_inet_pton_ipv6()
> 2744 MBEDTLS_PUT_UINT16_BE(group, addr, nonzero_groups);
> 2745 nonzero_groups++;
> 2746 if (*p == '\0') {
> 2747 break;
> 2748 } else if (*p == '.') {
> 2749 /* Don't accept IPv4 too early or late */
> >>> CID 510807: Control flow issues (DEADCODE)
> >>> Execution cannot reach the expression "zero_group_start == -1"
> inside this statement: "if ((nonzero_groups == 0 &&...".
> 2750 if ((nonzero_groups == 0 && zero_group_start ==
> -1) ||
> 2751 nonzero_groups >= 7) {
> 2752 break;
> 2753 }
> 2754
> 2755 /* Walk back to prior ':', then parse as
> IPv4-mapped */
>
> This one belongs to MbedTLS lib itself, I will send a separate patch to
the MbedTLS project.
[snip]
*** CID 510806: Control flow issues (DEADCODE)
> /lib/mbedtls/pkcs7_parser.c: 209 in authattrs_parse()
> 203 return -EINVAL;
> 204 }
> 205
> 206 p += seq_len;
> 207 }
> 208
> >>> CID 510806: Control flow issues (DEADCODE)
> >>> Execution cannot reach the expression "ret != -96" inside this
> statement: "if (ret && ret != -96)
> re...".
> 209 if (ret && ret != MBEDTLS_ERR_ASN1_OUT_OF_DATA)
> 210 return ret;
> 211
> 212 msg->have_authattrs = true;
> 213
> 214 /*
>
> I will post a patch to fix this defect .
[snip]
> *** CID 510798: Resource leaks (RESOURCE_LEAK)
> /lib/mbedtls/x509_cert_parser.c: 220 in x509_populate_signature_params()
> 214 }
> 215
> 216 ret = hash_calculate(s->hash_algo, ®ion, 1, s->digest);
> 217 if (!ret)
> 218 *sig = s;
> 219
> >>> CID 510798: Resource leaks (RESOURCE_LEAK)
> >>> Variable "s" going out of scope leaks the storage it points to.
> 220 return ret;
> 221
> 222 error_sig:
> 223 public_key_signature_free(s);
> 224 return ret;
> 225 }
>
> This is false-positive, we need to keep the memory pointed by 's' as the
signature
returned to the caller.
[snip]
*** CID 510796: Insecure data handling (TAINTED_SCALAR)
> /lib/mbedtls/external/mbedtls/library/rsa.c: 1316 in rsa_prepare_blinding()
> 1310 }
> 1311
> 1312 MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&ctx->Vf,
> ctx->len - 1, f_rng, p_rng));
> 1313
> 1314 /* Compute Vf^-1 as R * (R Vf)^-1 to avoid leaks from
> inv_mod. */
> 1315 MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&R, ctx->len
> - 1, f_rng, p_rng));
> >>> CID 510796: Insecure data handling (TAINTED_SCALAR)
> >>> Passing tainted expression "*ctx->Vf.p" to "mbedtls_mpi_mul_mpi",
> which uses it as an offset.
> 1316 MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&ctx->Vi, &ctx->Vf,
> &R));
> 1317 MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&ctx->Vi,
> &ctx->Vi, &ctx->N));
> 1318
> 1319 /* At this point, Vi is invertible mod N if and only
> if both Vf and R
> 1320 * are invertible mod N. If one of them isn't, we
> don't need to know
> 1321 * which one, we just loop and choose new values for
> both of them.
>
> This one belongs to MbedTLS lib itself, I will send a separate patch to
the MbedTLS project.
[snip]
*** CID 510794: Control flow issues (NO_EFFECT)
> /lib/mbedtls/x509_cert_parser.c: 78 in x509_populate_dn_name_string()
> 72 do {
> 73 name_str = kzalloc(len, GFP_KERNEL);
> 74 if (!name_str)
> 75 return NULL;
> 76
> 77 wb = mbedtls_x509_dn_gets(name_str, len, name);
> >>> CID 510794: Control flow issues (NO_EFFECT)
> >>> This less-than-zero comparison of an unsigned value is never true.
> "wb < 0UL".
> 78 if (wb < 0) {
> 79 pr_err("Get DN string failed, ret:-0x%04x\n",
> 80 (unsigned int)-wb);
> 81 kfree(name_str);
> 82 len = len * 2; /* Try with a bigger buffer */
> 83 }
>
> I will post a patch to fix this defect.
Thanks a lot for the report.
Regards,
Raymond
