[PATCH] lib/mbedtls: Do not use ERR_PTR with mbedTLS error codes

[Vsevolod Kozlov]

Hi Raymond,

On Mon, Jul 06, 2026 at 11:38:43AM -0400, Raymond Mao wrote:
> The direction is good, but the fixes do not match the commit message statement.
> In the commit message, only the MbedTLS native error code will be
> substituted, but the fixes below arbitrarily overridden all error
> codes including the standard U-Boot errnos.

Thank you for the review.

I have re-read the code better and I think it would be feasible to
convert return values of mbedtls functions to negative errnos right at
their respective call sites, because there are not too many of them.
The only exception is the mbedtls_asn1_get_tag function that gets
called quite many times in pkcs7_parser.c, but a wrapper function
could be added for it.

Do you think that is the right approach, or should perhaps these code
paths be converted away from ERR_PTR/IS_ERR entirely and just return
NULL on error, given that none of the callers do anything with the
error codes anyway?

Thank you and best wishes

> Please see my below inline comments.
> 
> > Signed-off-by: Vsevolod Kozlov <zaba at mm.st>
> > ---
> >  lib/mbedtls/pkcs7_parser.c     | 5 +++++
> >  lib/mbedtls/x509_cert_parser.c | 6 +++++-
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/mbedtls/pkcs7_parser.c b/lib/mbedtls/pkcs7_parser.c
> > index bf8ee17b5b8..2c85dcc9699 100644
> > --- a/lib/mbedtls/pkcs7_parser.c
> > +++ b/lib/mbedtls/pkcs7_parser.c
> > @@ -495,6 +495,11 @@ struct pkcs7_message *pkcs7_parse_message(const void *data, size_t datalen)
> >  parse_fail:
> >         mbedtls_pkcs7_free(&pkcs7_ctx);
> >         pkcs7_free_message(msg);
> > +
> > +       /* MbedTLS error codes lie beyond MAX_ERRNO and are not suitable
> > +        * for ERR_PTR, but can be useful for debugging. */
> > +       debug("mbedtls returned error: %x\n", ret);
> > +       ret = -EINVAL;
> 
> This collapses all non-zero failures to -EINVAL, including valid
> U-Boot errnos such as -ENOMEM.
> 
> >  out_no_msg:
> >         msg = ERR_PTR(ret);
> >         return msg;
> > diff --git a/lib/mbedtls/x509_cert_parser.c b/lib/mbedtls/x509_cert_parser.c
> > index e163e16b9bc..1c70a8fcdcb 100644
> > --- a/lib/mbedtls/x509_cert_parser.c
> > +++ b/lib/mbedtls/x509_cert_parser.c
> > @@ -425,7 +425,7 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
> >  {
> >         mbedtls_x509_crt mbedtls_cert;
> >         struct x509_certificate *cert = NULL;
> > -       long ret;
> > +       int ret;
> >
> >         /* Parse DER encoded certificate */
> >         mbedtls_x509_crt_init(&mbedtls_cert);
> > @@ -443,5 +443,9 @@ clean_up_ctx:
> >         if (!ret)
> >                 return cert;
> >
> > +       /* MbedTLS error codes lie beyond MAX_ERRNO and are not suitable
> > +        * for ERR_PTR, but can be useful for debugging. */
> > +       debug("mbedtls returned error: %x\n", ret);
> > +       ret = -EINVAL;
> 
> This collapses all non-zero failures to -EINVAL, including valid
> U-Boot errnos such as -ENOMEM.
> 
> Regards,
> Raymond
> 
> >         return ERR_PTR(ret);
> >  }
> > --
> > 2.47.3