[PATCH v2] lib/mbedtls: Do not use ERR_PTR with mbedTLS error values
Raymond Mao
raymondmaoca at gmail.com
Wed Jul 8 21:55:16 CEST 2026
Hi Vsevolod,
On Wed, Jul 8, 2026 at 1:30 PM Vsevolod Kozlov <zaba at mm.st> wrote:
>
> Hi Raymond,
>
> On Wed, Jul 08, 2026 at 01:06:39PM -0400, Raymond Mao wrote:
> > The reason for ERR_PTR in mbedtls port is to provide the same ABI for
> > EFI loader, compared with the legacy crypto libraries.
>
> Well, for example lib/efi_loader/efi_signature.c lines 321 and 382
> already use IS_ERR_OR_NULL with the return value of x509_cert_parse.
> It might be a good idea to make this more consistent regardless of
> mbedtls usage.
>
> > I would suggest to:
> > void *ERR_PTR_MBEDTLS(int ret) {
> > if (ret < -MAX_ERRNO) {
> > error("mbedtls internal error: %x\n", ret);
> > ret = -EIO;
> > }
> > return ERR_PTR(ret);
> > }
>
> My issue with this is that mbedTLS can also return error codes that
> overlap with negative errno, for example
> MBEDTLS_ERR_ERROR_GENERIC_ERROR has the value -1 and
> MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED has the value -110. So this is
> not guaranteed to report all errors originating from mbedTLS as such.
> But if you think that is acceptable, I can go ahead with that.
>
In the wrapper function, please map the known error code as possible
before "if (ret < -MAX_ERRNO)".
Raymond
> Thank you and best wishes
>
> >
> > Regards,
> > Raymond
> >
> >
> > > Thank you and best wishes
> > >
> > > > Regards,
> > > > Raymond
> > > >
> > > > > lib/mbedtls/pkcs7_parser.c | 48 ++++++++++++++++++++++++++--------
> > > > > lib/mbedtls/x509_cert_parser.c | 10 +++++--
> > > > > 2 files changed, 45 insertions(+), 13 deletions(-)
> > > > >
> > > > > diff --git a/lib/mbedtls/pkcs7_parser.c b/lib/mbedtls/pkcs7_parser.c
> > > > > index bf8ee17b5b8..6d255250e62 100644
> > > > > --- a/lib/mbedtls/pkcs7_parser.c
> > > > > +++ b/lib/mbedtls/pkcs7_parser.c
> > > > > @@ -29,6 +29,20 @@ static void pkcs7_free_sinfo_mbedtls_ctx(struct pkcs7_sinfo_mbedtls_ctx *ctx)
> > > > > }
> > > > > }
> > > > >
> > > > > +/* Convert mbedTLS error codes to negative errno to avoid creating
> > > > > + * out-of-range ERR_PTR values */
> > > > > +static int wrapped_asn1_get_tag(unsigned char **p,
> > > > > + const unsigned char *end,
> > > > > + size_t *len, int tag)
> > > > > +{
> > > > > + int ret = mbedtls_asn1_get_tag(p, end, len, tag);
> > > > > + if (ret) {
> > > > > + debug("mbedtls_asn1_get_tag() failed: %x\n", ret);
> > > > > + return -EINVAL;
> > > > > + }
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > /*
> > > > > * Parse Authenticate Attributes
> > > > > * TODO: Shall we consider to integrate decoding of authenticate attribute into
> > > > > @@ -105,30 +119,30 @@ static int authattrs_parse(struct pkcs7_message *msg, void *aa, size_t aa_len,
> > > > > unsigned char *inner_p;
> > > > > size_t seq_len = 0;
> > > > >
> > > > > - ret = mbedtls_asn1_get_tag(&p, end, &seq_len,
> > > > > + ret = wrapped_asn1_get_tag(&p, end, &seq_len,
> > > > > MBEDTLS_ASN1_CONTEXT_SPECIFIC |
> > > > > MBEDTLS_ASN1_CONSTRUCTED);
> > > > > if (ret)
> > > > > return ret;
> > > > >
> > > > > - while (!mbedtls_asn1_get_tag(&p, end, &seq_len,
> > > > > + while (!wrapped_asn1_get_tag(&p, end, &seq_len,
> > > > > MBEDTLS_ASN1_CONSTRUCTED |
> > > > > MBEDTLS_ASN1_SEQUENCE)) {
> > > > > inner_p = p;
> > > > > - ret = mbedtls_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > + ret = wrapped_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > MBEDTLS_ASN1_OID);
> > > > > if (ret)
> > > > > return ret;
> > > > >
> > > > > if (!MBEDTLS_OID_CMP_RAW(MBEDTLS_OID_PKCS9_CONTENTTYPE, inner_p, len)) {
> > > > > inner_p += len;
> > > > > - ret = mbedtls_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > + ret = wrapped_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > MBEDTLS_ASN1_CONSTRUCTED |
> > > > > MBEDTLS_ASN1_SET);
> > > > > if (ret)
> > > > > return ret;
> > > > >
> > > > > - ret = mbedtls_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > + ret = wrapped_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > MBEDTLS_ASN1_OID);
> > > > > if (ret)
> > > > > return ret;
> > > > > @@ -151,13 +165,13 @@ static int authattrs_parse(struct pkcs7_message *msg, void *aa, size_t aa_len,
> > > > > } else if (!MBEDTLS_OID_CMP_RAW(MBEDTLS_OID_PKCS9_MESSAGEDIGEST, inner_p,
> > > > > len)) {
> > > > > inner_p += len;
> > > > > - ret = mbedtls_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > + ret = wrapped_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > MBEDTLS_ASN1_CONSTRUCTED |
> > > > > MBEDTLS_ASN1_SET);
> > > > > if (ret)
> > > > > return ret;
> > > > >
> > > > > - ret = mbedtls_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > + ret = wrapped_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > MBEDTLS_ASN1_OCTET_STRING);
> > > > > if (ret)
> > > > > return ret;
> > > > > @@ -172,15 +186,18 @@ static int authattrs_parse(struct pkcs7_message *msg, void *aa, size_t aa_len,
> > > > > mbedtls_x509_time st;
> > > > >
> > > > > inner_p += len;
> > > > > - ret = mbedtls_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > + ret = wrapped_asn1_get_tag(&inner_p, p + seq_len, &len,
> > > > > MBEDTLS_ASN1_CONSTRUCTED |
> > > > > MBEDTLS_ASN1_SET);
> > > > > if (ret)
> > > > > return ret;
> > > > >
> > > > > ret = mbedtls_x509_get_time(&inner_p, p + seq_len, &st);
> > > > > - if (ret)
> > > > > + if (ret) {
> > > > > + debug("mbedtls_x509_get_time() failed: %x\n", ret);
> > > > > + ret = -EINVAL;
> > > > > return ret;
> > > > > + }
> > > > > sinfo->signing_time = x509_get_timestamp(&st);
> > > > >
> > > > > if (__test_and_set_bit(sinfo_has_signing_time, &sinfo->aa_set))
> > > > > @@ -287,8 +304,11 @@ static int x509_populate_sinfo(struct pkcs7_message *msg,
> > > > > * info. Assume that we are using RSA.
> > > > > */
> > > > > ret = mbedtls_oid_get_md_alg(&mb_sinfo->alg_identifier, &md_alg);
> > > > > - if (ret)
> > > > > + if (ret) {
> > > > > + debug("mbedtls_oid_get_md_alg() failed: %x\n", ret);
> > > > > + ret = -EINVAL;
> > > > > goto out_err_sinfo;
> > > > > + }
> > > > > s->pkey_algo = "rsa";
> > > > >
> > > > > /* Translate the hash algorithm */
> > > > > @@ -451,8 +471,14 @@ struct pkcs7_message *pkcs7_parse_message(const void *data, size_t datalen)
> > > > > mbedtls_pkcs7_init(&pkcs7_ctx);
> > > > > ret = mbedtls_pkcs7_parse_der(&pkcs7_ctx, data, datalen);
> > > > > /* Check if it is a PKCS#7 message with signed data */
> > > > > - if (ret != MBEDTLS_PKCS7_SIGNED_DATA)
> > > > > + if (ret != MBEDTLS_PKCS7_SIGNED_DATA) {
> > > > > + debug("mbedtls_pkcs7_parse_der() failed: %x\n", ret);
> > > > > +
> > > > > + /* MbedTLS error codes lie beyond MAX_ERRNO and are not
> > > > > + * suitable for ERR_PTR. */
> > > > > + ret = -EINVAL;
> > > > > goto parse_fail;
> > > > > + }
> > > > >
> > > > > /* Assume that we are using PKCS#7, not CMS ver 3 */
> > > > > msg->version = 1; /* 1 for [PKCS#7 or CMS ver 1] */
> > > > > diff --git a/lib/mbedtls/x509_cert_parser.c b/lib/mbedtls/x509_cert_parser.c
> > > > > index e163e16b9bc..181727e1eda 100644
> > > > > --- a/lib/mbedtls/x509_cert_parser.c
> > > > > +++ b/lib/mbedtls/x509_cert_parser.c
> > > > > @@ -425,13 +425,19 @@ 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);
> > > > > ret = mbedtls_x509_crt_parse_der(&mbedtls_cert, data, datalen);
> > > > > - if (ret)
> > > > > + if (ret) {
> > > > > + debug("mbedtls_x509_crt_parse_der() failed: %x\n", ret);
> > > > > +
> > > > > + /* MbedTLS error codes lie beyond MAX_ERRNO and are not
> > > > > + * suitable for ERR_PTR. */
> > > > > + ret = -EINVAL;
> > > > > goto clean_up_ctx;
> > > > > + }
> > > > >
> > > > > /* Populate x509_certificate from mbedtls_x509_crt */
> > > > > ret = x509_populate_cert(&mbedtls_cert, &cert);
> > > > > --
> > > > > 2.47.3
> > > > >
More information about the U-Boot
mailing list