Re: [PATCH] crypto: asymmetric_keys: fix uninitialized pointers with free attr

[ally heev]

On Tue, Nov 11, 2025 at 6:42 PM Ignat Korchagin <ignat@xxxxxxxxxxxxxx> wrote:
[...]
> Should this be just initialized to NULL instead of moving the declaration?
>
> > -       struct x509_parse_context *ctx __free(kfree) = NULL;
>
> This pointer seems initialized. Is there still a problem?
>
> >         struct asymmetric_key_id *kid;
> >         long ret;
> >
> > -       cert = kzalloc(sizeof(struct x509_certificate), GFP_KERNEL);
> > +       struct x509_certificate *cert __free(x509_free_certificate) = kzalloc(
> > +               sizeof(struct x509_certificate), GFP_KERNEL);
> > +
> >         if (!cert)
> >                 return ERR_PTR(-ENOMEM);
> >         cert->pub = kzalloc(sizeof(struct public_key), GFP_KERNEL);
> > @@ -74,7 +74,10 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
> >         cert->sig = kzalloc(sizeof(struct public_key_signature), GFP_KERNEL);
> >         if (!cert->sig)
> >                 return ERR_PTR(-ENOMEM);
> > -       ctx = kzalloc(sizeof(struct x509_parse_context), GFP_KERNEL);
> > +
> > +       struct x509_parse_context *ctx __free(kfree) = kzalloc(
> > +               sizeof(struct x509_parse_context), GFP_KERNEL);
> > +
> >         if (!ctx)
> >                 return ERR_PTR(-ENOMEM);
> >
> > diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> > index 8409d7d36cb4f3582e15f9ee4d25f302b3b29358..818c9ab5d63940ff62c21666fd549d3a1ff07e67 100644
> > --- a/crypto/asymmetric_keys/x509_public_key.c
> > +++ b/crypto/asymmetric_keys/x509_public_key.c
> > @@ -148,13 +148,13 @@ int x509_check_for_self_signed(struct x509_certificate *cert)
> >   */
> >  static int x509_key_preparse(struct key_preparsed_payload *prep)
> >  {
> > -       struct x509_certificate *cert __free(x509_free_certificate);
>
> And here: should we just initialize this to NULL?
>
> > -       struct asymmetric_key_ids *kids __free(kfree) = NULL;
> > -       char *p, *desc __free(kfree) = NULL;
>
> Same here: these two pointers are initialized.
>
> > +       char *p;
> >         const char *q;
> >         size_t srlen, sulen;
> >
> > -       cert = x509_cert_parse(prep->data, prep->datalen);
> > +       struct x509_certificate *cert __free(x509_free_certificate) =
> > +               x509_cert_parse(prep->data, prep->datalen);
> > +
> >         if (IS_ERR(cert))
> >                 return PTR_ERR(cert);
> >
> > @@ -187,7 +187,7 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
> >                 q = cert->raw_serial;
> >         }
> >
> > -       desc = kmalloc(sulen + 2 + srlen * 2 + 1, GFP_KERNEL);
> > +       char *desc __free(kfree) = kmalloc(sulen + 2 + srlen * 2 + 1, GFP_KERNEL);
> >         if (!desc)
> >                 return -ENOMEM;
> >         p = memcpy(desc, cert->subject, sulen);
> > @@ -197,7 +197,9 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
> >         p = bin2hex(p, q, srlen);
> >         *p = 0;
> >
> > -       kids = kmalloc(sizeof(struct asymmetric_key_ids), GFP_KERNEL);
> > +       struct asymmetric_key_ids *kids __free(kfree) = kmalloc(
> > +               sizeof(struct asymmetric_key_ids), GFP_KERNEL);
> > +
> >         if (!kids)
> >                 return -ENOMEM;
> >         kids->id[0] = cert->id;
> >
> > ---
> > base-commit: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
> > change-id: 20251105-aheev-uninitialized-free-attr-crypto-bc94ec1b2253
> >
> > Best regards,
> > --
> > Ally Heev <allyheev@xxxxxxxxx>
> >
>
> Ignat

initializing to NULL at the declaration itself seems to be the right
solution here, as it conforms to the `variable declarations at the
top` rule too. I will send a new version with the changes

Regards,
Ally