| Message ID | 20241016234829.3836359-1-raymond.mao@linaro.org |
|---|---|
| State | Accepted |
| Commit | 7f453771528160f0401a8cb7cd871c32e56f63f2 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | mbedtls: fix defects in coverity scan | expand |
On Wed, 16 Oct 2024 16:48:26 -0700, Raymond Mao wrote: > Fixes of unreleased buffer, deadcode and wrong variable type detected > by coverity scan. > > Addresses-Coverity-ID: 510809: Resource leaks (RESOURCE_LEAK) > Addresses-Coverity-ID: 510806: Control flow issues (DEADCODE) > Addresses-Coverity-ID: 510794 Control flow issues (NO_EFFECT) > > [...] Applied to u-boot/master, thanks!
On Thu, 17 Oct 2024 at 00:49, Raymond Mao <raymond.mao@linaro.org> wrote: > > Fixes of unreleased buffer, deadcode and wrong variable type detected > by coverity scan. > > Addresses-Coverity-ID: 510809: Resource leaks (RESOURCE_LEAK) > Addresses-Coverity-ID: 510806: Control flow issues (DEADCODE) > Addresses-Coverity-ID: 510794 Control flow issues (NO_EFFECT) I think it makes sense to reference upstream commits/PRs for these sort of things moving forward, Tom maybe we need a policy around the third party libraries now we have a few more. Peter > Signed-off-by: Raymond Mao <raymond.mao@linaro.org> > --- > lib/mbedtls/pkcs7_parser.c | 7 +++---- > lib/mbedtls/x509_cert_parser.c | 2 +- > 2 files changed, 4 insertions(+), 5 deletions(-) > > diff --git a/lib/mbedtls/pkcs7_parser.c b/lib/mbedtls/pkcs7_parser.c > index 69ca784858e..ecfcc46edfa 100644 > --- a/lib/mbedtls/pkcs7_parser.c > +++ b/lib/mbedtls/pkcs7_parser.c > @@ -206,9 +206,6 @@ static int authattrs_parse(struct pkcs7_message *msg, void *aa, size_t aa_len, > p += seq_len; > } > > - if (ret && ret != MBEDTLS_ERR_ASN1_OUT_OF_DATA) > - return ret; > - > msg->have_authattrs = true; > > /* > @@ -361,8 +358,10 @@ static int x509_populate_sinfo(struct pkcs7_message *msg, > signed_info->sig = s; > > /* Save the Authenticate Attributes data if exists */ > - if (!mb_sinfo->authattrs.data || !mb_sinfo->authattrs.data_len) > + if (!mb_sinfo->authattrs.data || !mb_sinfo->authattrs.data_len) { > + kfree(mctx); > goto no_authattrs; > + } > > mctx->authattrs_data = kmemdup(mb_sinfo->authattrs.data, > mb_sinfo->authattrs.data_len, > diff --git a/lib/mbedtls/x509_cert_parser.c b/lib/mbedtls/x509_cert_parser.c > index cb42018695c..e163e16b9bc 100644 > --- a/lib/mbedtls/x509_cert_parser.c > +++ b/lib/mbedtls/x509_cert_parser.c > @@ -66,7 +66,7 @@ time64_t x509_get_timestamp(const mbedtls_x509_time *x509_time) > static char *x509_populate_dn_name_string(const mbedtls_x509_name *name) > { > size_t len = 256; > - size_t wb; > + int wb; > char *name_str; > > do { > -- > 2.25.1 >
On Sat, Oct 19, 2024 at 10:37:38AM +0100, Peter Robinson wrote: > On Thu, 17 Oct 2024 at 00:49, Raymond Mao <raymond.mao@linaro.org> wrote: > > > > Fixes of unreleased buffer, deadcode and wrong variable type detected > > by coverity scan. > > > > Addresses-Coverity-ID: 510809: Resource leaks (RESOURCE_LEAK) > > Addresses-Coverity-ID: 510806: Control flow issues (DEADCODE) > > Addresses-Coverity-ID: 510794 Control flow issues (NO_EFFECT) > > I think it makes sense to reference upstream commits/PRs for these > sort of things moving forward, Tom maybe we need a policy around the > third party libraries now we have a few more. We should have a policy about what to do with issues found in external libraries (Raymond is going to work with upstream) which is likely just formally saying we'll report issues when found and depending on severity cherry-pick the fixes once resolved. But these are issues in our glue code, rather than upstream.
diff --git a/lib/mbedtls/pkcs7_parser.c b/lib/mbedtls/pkcs7_parser.c index 69ca784858e..ecfcc46edfa 100644 --- a/lib/mbedtls/pkcs7_parser.c +++ b/lib/mbedtls/pkcs7_parser.c @@ -206,9 +206,6 @@ static int authattrs_parse(struct pkcs7_message *msg, void *aa, size_t aa_len, p += seq_len; } - if (ret && ret != MBEDTLS_ERR_ASN1_OUT_OF_DATA) - return ret; - msg->have_authattrs = true; /* @@ -361,8 +358,10 @@ static int x509_populate_sinfo(struct pkcs7_message *msg, signed_info->sig = s; /* Save the Authenticate Attributes data if exists */ - if (!mb_sinfo->authattrs.data || !mb_sinfo->authattrs.data_len) + if (!mb_sinfo->authattrs.data || !mb_sinfo->authattrs.data_len) { + kfree(mctx); goto no_authattrs; + } mctx->authattrs_data = kmemdup(mb_sinfo->authattrs.data, mb_sinfo->authattrs.data_len, diff --git a/lib/mbedtls/x509_cert_parser.c b/lib/mbedtls/x509_cert_parser.c index cb42018695c..e163e16b9bc 100644 --- a/lib/mbedtls/x509_cert_parser.c +++ b/lib/mbedtls/x509_cert_parser.c @@ -66,7 +66,7 @@ time64_t x509_get_timestamp(const mbedtls_x509_time *x509_time) static char *x509_populate_dn_name_string(const mbedtls_x509_name *name) { size_t len = 256; - size_t wb; + int wb; char *name_str; do {
Fixes of unreleased buffer, deadcode and wrong variable type detected by coverity scan. Addresses-Coverity-ID: 510809: Resource leaks (RESOURCE_LEAK) Addresses-Coverity-ID: 510806: Control flow issues (DEADCODE) Addresses-Coverity-ID: 510794 Control flow issues (NO_EFFECT) Signed-off-by: Raymond Mao <raymond.mao@linaro.org> --- lib/mbedtls/pkcs7_parser.c | 7 +++---- lib/mbedtls/x509_cert_parser.c | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-)