[PATCH v2] crypto: asymmetric_keys: fix uninitialized pointers with free attribute

[PATCH v2] crypto: asymmetric_keys: fix uninitialized pointers with free attribute

[Ally Heev]

Uninitialized pointers with `__free` attribute can cause undefined
behavior as the memory assigned randomly to the pointer is freed
automatically when the pointer goes out of scope.

crypto/asymmetric_keys doesn't have any bugs related to this as of now,
but, it is better to initialize and assign pointers with `__free`
attribute in one statement to ensure proper scope-based cleanup

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
Signed-off-by: Ally Heev <allyheev@gmail.com>
---
Changes in v2:
- moved declarations to the top and initialized them with NULL
- Link to v1: https://lore.kernel.org/r/20251105-aheev-uninitialized-free-attr-crypto-v1-1-83da1e10e8c4@gmail.com
---
 crypto/asymmetric_keys/x509_cert_parser.c | 2 +-
 crypto/asymmetric_keys/x509_public_key.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 8df3fa60a44f80fbd71af17faeca2e92b6cc03ce..b37cae914987b69c996d6559058c00f13c92b5b9 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -60,7 +60,7 @@ EXPORT_SYMBOL_GPL(x509_free_certificate);
  */
 struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
 {
-	struct x509_certificate *cert __free(x509_free_certificate);
+	struct x509_certificate *cert __free(x509_free_certificate) = NULL;
 	struct x509_parse_context *ctx __free(kfree) = NULL;
 	struct asymmetric_key_id *kid;
 	long ret;
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 8409d7d36cb4f3582e15f9ee4d25f302b3b29358..12e3341e806b8db93803325a96a3821fd5d0a9f0 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -148,7 +148,7 @@ 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);
+	struct x509_certificate *cert __free(x509_free_certificate) = NULL;
 	struct asymmetric_key_ids *kids __free(kfree) = NULL;
 	char *p, *desc __free(kfree) = NULL;
 	const char *q;

---
base-commit: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
change-id: 20251105-aheev-uninitialized-free-attr-crypto-bc94ec1b2253

Best regards,
-- 
Ally Heev <allyheev@gmail.com>

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

[Ignat Korchagin]

On Tue, Nov 11, 2025 at 1:36 PM Ally Heev <allyheev@gmail.com> wrote:
>
> Uninitialized pointers with `__free` attribute can cause undefined
> behavior as the memory assigned randomly to the pointer is freed
> automatically when the pointer goes out of scope.
>
> crypto/asymmetric_keys doesn't have any bugs related to this as of now,
> but, it is better to initialize and assign pointers with `__free`
> attribute in one statement to ensure proper scope-based cleanup
>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
> Signed-off-by: Ally Heev <allyheev@gmail.com>

Reviewed-by: Ignat Korchagin <ignat@cloudflare.com>

> ---
> Changes in v2:
> - moved declarations to the top and initialized them with NULL
> - Link to v1: https://lore.kernel.org/r/20251105-aheev-uninitialized-free-attr-crypto-v1-1-83da1e10e8c4@gmail.com
> ---
>  crypto/asymmetric_keys/x509_cert_parser.c | 2 +-
>  crypto/asymmetric_keys/x509_public_key.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
> index 8df3fa60a44f80fbd71af17faeca2e92b6cc03ce..b37cae914987b69c996d6559058c00f13c92b5b9 100644
> --- a/crypto/asymmetric_keys/x509_cert_parser.c
> +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> @@ -60,7 +60,7 @@ EXPORT_SYMBOL_GPL(x509_free_certificate);
>   */
>  struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
>  {
> -       struct x509_certificate *cert __free(x509_free_certificate);
> +       struct x509_certificate *cert __free(x509_free_certificate) = NULL;
>         struct x509_parse_context *ctx __free(kfree) = NULL;
>         struct asymmetric_key_id *kid;
>         long ret;
> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> index 8409d7d36cb4f3582e15f9ee4d25f302b3b29358..12e3341e806b8db93803325a96a3821fd5d0a9f0 100644
> --- a/crypto/asymmetric_keys/x509_public_key.c
> +++ b/crypto/asymmetric_keys/x509_public_key.c
> @@ -148,7 +148,7 @@ 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);
> +       struct x509_certificate *cert __free(x509_free_certificate) = NULL;
>         struct asymmetric_key_ids *kids __free(kfree) = NULL;
>         char *p, *desc __free(kfree) = NULL;
>         const char *q;
>
> ---
> base-commit: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
> change-id: 20251105-aheev-uninitialized-free-attr-crypto-bc94ec1b2253
>
> Best regards,
> --
> Ally Heev <allyheev@gmail.com>
>

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

[Herbert Xu]

On Tue, Nov 11, 2025 at 07:06:29PM +0530, Ally Heev wrote:
> Uninitialized pointers with `__free` attribute can cause undefined
> behavior as the memory assigned randomly to the pointer is freed
> automatically when the pointer goes out of scope.
> 
> crypto/asymmetric_keys doesn't have any bugs related to this as of now,
> but, it is better to initialize and assign pointers with `__free`
> attribute in one statement to ensure proper scope-based cleanup
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
> Signed-off-by: Ally Heev <allyheev@gmail.com>
> ---
> Changes in v2:
> - moved declarations to the top and initialized them with NULL
> - Link to v1: https://lore.kernel.org/r/20251105-aheev-uninitialized-free-attr-crypto-v1-1-83da1e10e8c4@gmail.com
> ---
>  crypto/asymmetric_keys/x509_cert_parser.c | 2 +-
>  crypto/asymmetric_keys/x509_public_key.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Krzysztof Kozlowski]

On 11/11/2025 14:36, Ally Heev wrote:
> Uninitialized pointers with `__free` attribute can cause undefined
> behavior as the memory assigned randomly to the pointer is freed
> automatically when the pointer goes out of scope.
> 
> crypto/asymmetric_keys doesn't have any bugs related to this as of now,
> but, it is better to initialize and assign pointers with `__free`
> attribute in one statement to ensure proper scope-based cleanup
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
> Signed-off-by: Ally Heev <allyheev@gmail.com>
> ---
> Changes in v2:
> - moved declarations to the top and initialized them with NULL

Why? This is not the syntax we want for cleanup.h. Either initialize it
with proper constructor or don't use cleanup.h.


Best regards,
Krzysztof

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

[ally heev]

On Sat, 2025-11-22 at 15:25 +0100, Krzysztof Kozlowski wrote:
> On 11/11/2025 14:36, Ally Heev wrote:
> > Uninitialized pointers with `__free` attribute can cause undefined
> > behavior as the memory assigned randomly to the pointer is freed
> > automatically when the pointer goes out of scope.
> > 
> > crypto/asymmetric_keys doesn't have any bugs related to this as of now,
> > but, it is better to initialize and assign pointers with `__free`
> > attribute in one statement to ensure proper scope-based cleanup
> > 
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
> > Signed-off-by: Ally Heev <allyheev@gmail.com>
> > ---
> > Changes in v2:
> > - moved declarations to the top and initialized them with NULL
> 
> Why? This is not the syntax we want for cleanup.h. Either initialize it
> with proper constructor or don't use cleanup.h.
> 
> 
> Best regards,
> Krzysztof

This is the only one I missed reverting :(
(after the disc https://lore.kernel.org/lkml/58fd478f408a34b578ee8d949c5c4b4da4d4f41d.camel@HansenPartnership.com/)

Regards,
Ally