New Defects reported by Coverity Scan for Das U-Boot

[Raymond Mao]

Hi Tom,

On Tue, 15 Oct 2024 at 23:47, Tom Rini <trini at konsulko.com> wrote:

> Hey all, here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin at coverity.com>
> Date: Tue, Oct 15, 2024 at 5:06 PM
> Subject: New Defects reported by Coverity Scan for Das U-Boot
> To: <tom.rini at 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, &region, 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