linux-kernel - Re: [PATCH] checkpatch: add uninitialized pointer with _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <68f7b830ec21a_10e910070@dwillia2-mobl4.notmuch>
Date: Tue, 21 Oct 2025 09:43:28 -0700
From: <dan.j.williams@...el.com>
To: Ally Heev <allyheev@...il.com>, Dwaipayan Ray <dwaipayanray1@...il.com>,
	Lukas Bulwahn <lukas.bulwahn@...il.com>, Joe Perches <joe@...ches.com>,
	Jonathan Corbet <corbet@....net>, Andy Whitcroft <apw@...onical.com>
CC: <workflows@...r.kernel.org>, <linux-doc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, Dan Carpenter <dan.carpenter@...aro.org>,
	David Hunter <david.hunter.linux@...il.com>, Shuah Khan
	<skhan@...uxfoundation.org>, Ally Heev <allyheev@...il.com>
Subject: Re: [PATCH] checkpatch: add uninitialized pointer with __free
 attribute check

Ally Heev wrote:
> uninitialized pointers with __free attribute can cause undefined
> behaviour as the memory allocated to the pointer is freed
> automatically when the pointer goes out of scope.
> add check in checkpatch to detect such issues
> 
> Suggested-by: Dan Carpenter <dan.carpenter@...aro.org>
> Link: https://lore.kernel.org/all/8a4c0b43-cf63-400d-b33d-d9c447b7e0b9@suswa.mountain/
> Signed-off-by: Ally Heev <allyheev@...il.com>
> ---
> Test:
> ran checkpatch.pl before and after the change on 
> crypto/asymmetric_keys/x509_public_key.c, which has
> both initialized and uninitialized pointers
> ---
>  Documentation/dev-tools/checkpatch.rst | 5 +++++
>  scripts/checkpatch.pl                  | 6 ++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> index d5c47e560324fb2399a5b1bc99c891ed1de10535..1a304bf38bcd27e50bbb7cd4383b07ac54d20b0a 100644
> --- a/Documentation/dev-tools/checkpatch.rst
> +++ b/Documentation/dev-tools/checkpatch.rst
> @@ -1009,6 +1009,11 @@ Functions and Variables
>  
>        return bar;
>  
> +  **UNINITIALIZED_PTR_WITH_FREE**
> +    Pointers with __free attribute should be initialized. Not doing so
> +    may lead to undefined behavior as the memory allocated (garbage,
> +    in case not initialized) to the pointer is freed automatically
> +    when the pointer goes out of scope.
>  
>  Permissions
>  -----------
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 92669904eecc7a8d2afd3f2625528e02b6d17cd6..33cb09843431bebef72a4f5daab3a5d321bcb911 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -7721,6 +7721,12 @@ sub process {
>  				ERROR("MISSING_SENTINEL", "missing sentinel in ID array\n" . "$here\n$stat\n");
>  			}
>  		}
> +
> +# check for uninitialized pointers with __free attribute
> +		if ($line =~ /\s*$Type\s*($Ident)\s+__free\s*\(\s*$Ident\s*\)\s*;/) {
> +			WARN("UNINITIALIZED_PTR_WITH_FREE",
> +			      "pointer '$1' with __free attribute should be initialized\n" . $herecurr);

Looks good to me, but I why WARN and not ERROR? Is there ever a valid
reason to ignore this warning?

I would go futher and suggest that the pattern of:

	type foo __free(free_foo) = NULL;

...be made into a warning because that easily leads to situations where
declaration order is out of sync with allocation order. I.e. can be made
technically correct, but at a level of cleverness that undermines the
benefit.

With or without the conversion to ERROR() for the above,

Acked-by: Dan Williams <dan.j.williams@...el.com>