[PATCH v2] lib/mbedtls: Do not use ERR_PTR with mbedTLS error values
Raymond Mao
raymondmaoca at gmail.com
Tue Jul 7 23:23:08 CEST 2026
Hi Vsevolod,
On Tue, Jul 7, 2026 at 1:01 PM Vsevolod Kozlov <zaba at mm.st> wrote:
>
> MbedTLS functions usually return zero for success and negative error
> codes to indicate failure, but its error codes lie way outside the range
> of -MAX_ERRNO, so using them with ERR_PTR creates values that do not
> pass the IS_ERR check and lead to crashes.
>
> Signed-off-by: Vsevolod Kozlov <zaba at mm.st>
> ---
> Changes since v1: Convert return values of all mbedtls functions at
> their call sites instead of trying to do it right before returning
> ERR_PTR.
>
There might be some misunderstandings. I meant that there were two options:
1. Like what you mentioned, update ERR_PTR/IS_ERR.
2. Have a shim wrapper on top of ERR_PTR/IS_ERR for lib/mbedtls use only.
But having wrappers on those mbedtls_ functions is not a good idea,
especially explicitly with a prefix wrapped_.
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