X.509: Introduce scope-based x509_certificate allocation

Add a DEFINE_FREE() clause for x509_certificate structs and use it in
x509_cert_parse() and x509_key_preparse().  These are the only functions
where scope-based x509_certificate allocation currently makes sense.
A third user will be introduced with the forthcoming SPDM library
(Security Protocol and Data Model) for PCI device authentication.

Unlike most other DEFINE_FREE() clauses, this one checks for IS_ERR()
instead of NULL before calling x509_free_certificate() at end of scope.
That's because the "constructor" of x509_certificate structs,
x509_cert_parse(), returns a valid pointer or an ERR_PTR(), but never
NULL.

Comparing the Assembler output before/after has shown they are identical,
save for the fact that gcc-12 always generates two return paths when
__cleanup() is used, one for the success case and one for the error case.

In x509_cert_parse(), add a hint for the compiler that kzalloc() never
returns an ERR_PTR().  Otherwise the compiler adds a gratuitous IS_ERR()
check on return.  Introduce an assume() macro for this which can be
re-used elsewhere in the kernel to provide hints for the compiler.

Suggested-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Link: https://lore.kernel.org/all/20231003153937.000034ca@Huawei.com/
Link: https://lwn.net/Articles/934679/
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 964208d1a35fb9d0772b51e3399425f32223f573..a814e5f136e2cb4a6f6571dc8baf8a746e99064f 100644 (file)
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -60,24 +60,24 @@ EXPORT_SYMBOL_GPL(x509_free_certificate);
  */
 struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
 {
-       struct x509_certificate *cert;
-       struct x509_parse_context *ctx;
+       struct x509_certificate *cert __free(x509_free_certificate);
+       struct x509_parse_context *ctx __free(kfree) = NULL;
        struct asymmetric_key_id *kid;
        long ret;
 
-       ret = -ENOMEM;
        cert = kzalloc(sizeof(struct x509_certificate), GFP_KERNEL);
+       assume(!IS_ERR(cert)); /* Avoid gratuitous IS_ERR() check on return */
        if (!cert)
-               goto error_no_cert;
+               return ERR_PTR(-ENOMEM);
        cert->pub = kzalloc(sizeof(struct public_key), GFP_KERNEL);
        if (!cert->pub)
-               goto error_no_ctx;
+               return ERR_PTR(-ENOMEM);
        cert->sig = kzalloc(sizeof(struct public_key_signature), GFP_KERNEL);
        if (!cert->sig)
-               goto error_no_ctx;
+               return ERR_PTR(-ENOMEM);
        ctx = kzalloc(sizeof(struct x509_parse_context), GFP_KERNEL);
        if (!ctx)
-               goto error_no_ctx;
+               return ERR_PTR(-ENOMEM);
 
        ctx->cert = cert;
        ctx->data = (unsigned long)data;
@@ -85,7 +85,7 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
        /* Attempt to decode the certificate */
        ret = asn1_ber_decoder(&x509_decoder, ctx, data, datalen);
        if (ret < 0)
-               goto error_decode;
+               return ERR_PTR(ret);
 
        /* Decode the AuthorityKeyIdentifier */
        if (ctx->raw_akid) {
@@ -95,20 +95,19 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
                                       ctx->raw_akid, ctx->raw_akid_size);
                if (ret < 0) {
                        pr_warn("Couldn't decode AuthKeyIdentifier\n");
-                       goto error_decode;
+                       return ERR_PTR(ret);
                }
        }
 
-       ret = -ENOMEM;
        cert->pub->key = kmemdup(ctx->key, ctx->key_size, GFP_KERNEL);
        if (!cert->pub->key)
-               goto error_decode;
+               return ERR_PTR(-ENOMEM);
 
        cert->pub->keylen = ctx->key_size;
 
        cert->pub->params = kmemdup(ctx->params, ctx->params_size, GFP_KERNEL);
        if (!cert->pub->params)
-               goto error_decode;
+               return ERR_PTR(-ENOMEM);
 
        cert->pub->paramlen = ctx->params_size;
        cert->pub->algo = ctx->key_algo;
@@ -116,33 +115,23 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
        /* Grab the signature bits */
        ret = x509_get_sig_params(cert);
        if (ret < 0)
-               goto error_decode;
+               return ERR_PTR(ret);
 
        /* Generate cert issuer + serial number key ID */
        kid = asymmetric_key_generate_id(cert->raw_serial,
                                         cert->raw_serial_size,
                                         cert->raw_issuer,
                                         cert->raw_issuer_size);
-       if (IS_ERR(kid)) {
-               ret = PTR_ERR(kid);
-               goto error_decode;
-       }
+       if (IS_ERR(kid))
+               return ERR_CAST(kid);
        cert->id = kid;
 
        /* Detect self-signed certificates */
        ret = x509_check_for_self_signed(cert);
        if (ret < 0)
-               goto error_decode;
-
-       kfree(ctx);
-       return cert;
+               return ERR_PTR(ret);
 
-error_decode:
-       kfree(ctx);
-error_no_ctx:
-       x509_free_certificate(cert);
-error_no_cert:
-       return ERR_PTR(ret);
+       return_ptr(cert);
 }
 EXPORT_SYMBOL_GPL(x509_cert_parse);
 

diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 97a886cbe01c3de4271eddbe6e28cf9ff7432389..0688c222806b0160f4749951d14e16d59cfe4970 100644 (file)
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -5,6 +5,7 @@
  * Written by David Howells (dhowells@redhat.com)
  */
 
+#include <linux/cleanup.h>
 #include <linux/time.h>
 #include <crypto/public_key.h>
 #include <keys/asymmetric-type.h>
@@ -44,6 +45,8 @@ struct x509_certificate {
  * x509_cert_parser.c
  */
 extern void x509_free_certificate(struct x509_certificate *cert);
+DEFINE_FREE(x509_free_certificate, struct x509_certificate *,
+           if (!IS_ERR(_T)) x509_free_certificate(_T))
 extern struct x509_certificate *x509_cert_parse(const void *data, size_t datalen);
 extern int x509_decode_time(time64_t *_t,  size_t hdrlen,
                            unsigned char tag,

diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 6a4f00be22fc10b55b500fd0d9e1b4ffae50fc48..00ac7159fba2beab40c093b9780152193c28bae6 100644 (file)
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -161,12 +161,11 @@ not_self_signed:
  */
 static int x509_key_preparse(struct key_preparsed_payload *prep)
 {
-       struct asymmetric_key_ids *kids;
-       struct x509_certificate *cert;
+       struct x509_certificate *cert __free(x509_free_certificate);
+       struct asymmetric_key_ids *kids __free(kfree) = NULL;
+       char *p, *desc __free(kfree) = NULL;
        const char *q;
        size_t srlen, sulen;
-       char *desc = NULL, *p;
-       int ret;
 
        cert = x509_cert_parse(prep->data, prep->datalen);
        if (IS_ERR(cert))
@@ -188,9 +187,8 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
        }
 
        /* Don't permit addition of blacklisted keys */
-       ret = -EKEYREJECTED;
        if (cert->blacklisted)
-               goto error_free_cert;
+               return -EKEYREJECTED;
 
        /* Propose a description */
        sulen = strlen(cert->subject);
@@ -202,10 +200,9 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
                q = cert->raw_serial;
        }
 
-       ret = -ENOMEM;
        desc = kmalloc(sulen + 2 + srlen * 2 + 1, GFP_KERNEL);
        if (!desc)
-               goto error_free_cert;
+               return -ENOMEM;
        p = memcpy(desc, cert->subject, sulen);
        p += sulen;
        *p++ = ':';
@@ -215,16 +212,14 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 
        kids = kmalloc(sizeof(struct asymmetric_key_ids), GFP_KERNEL);
        if (!kids)
-               goto error_free_desc;
+               return -ENOMEM;
        kids->id[0] = cert->id;
        kids->id[1] = cert->skid;
        kids->id[2] = asymmetric_key_generate_id(cert->raw_subject,
                                                 cert->raw_subject_size,
                                                 "", 0);
-       if (IS_ERR(kids->id[2])) {
-               ret = PTR_ERR(kids->id[2]);
-               goto error_free_kids;
-       }
+       if (IS_ERR(kids->id[2]))
+               return PTR_ERR(kids->id[2]);
 
        /* We're pinning the module by being linked against it */
        __module_get(public_key_subtype.owner);
@@ -242,15 +237,7 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
        cert->sig = NULL;
        desc = NULL;
        kids = NULL;
-       ret = 0;
-
-error_free_kids:
-       kfree(kids);
-error_free_desc:
-       kfree(desc);
-error_free_cert:
-       x509_free_certificate(cert);
-       return ret;
+       return 0;
 }
 
 static struct asymmetric_key_parser x509_key_parser = {

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index c00cc6c0878a1e173701a6267ac062fa3d41b790..53666eb19909d21e7ed63365c8c4381108ab359c 100644 (file)
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -148,6 +148,8 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 } while (0)
 #endif
 
+#define assume(cond) do { if (!(cond)) __builtin_unreachable(); } while (0)
+
 /*
  * KENTRY - kernel entry point
  * This can be used to annotate symbols (functions or data) that are used