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

[Raymond Mao]

Hi Vsevolod,

On Mon, Jul 6, 2026 at 1:19 PM Vsevolod Kozlov <zaba at mm.st> wrote:
>
> 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?
>

Yes. I prefer to update ERR_PTR/IS_ERR. Or at least, to have a wrapper
on top of ERR_PTR/IS_ERR for MbedTLS use only. But I think the former
is better.

Raymond


> 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