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 <[email protected]> --- 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. 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
