workflows.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Ally Heev <allyheev@gmail.com>,
	Dwaipayan Ray <dwaipayanray1@gmail.com>,
	Lukas Bulwahn <lukas.bulwahn@gmail.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Andy Whitcroft <apw@canonical.com>,
	 Andrew Morton <akpm@linux-foundation.org>
Cc: workflows@vger.kernel.org, linux-doc@vger.kernel.org,
	 linux-kernel@vger.kernel.org,
	Dan Carpenter <dan.carpenter@linaro.org>,
	David Hunter <david.hunter.linux@gmail.com>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Viresh Kumar	 <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
	Stephen Boyd <sboyd@kernel.org>,
	 linux-pm <linux-pm@vger.kernel.org>,
	dan.j.williams@intel.com,
	Geert Uytterhoeven	 <geert@linux-m68k.org>,
	James Bottomley <James.Bottomley@hansenpartnership.com>,
	 Krzysztof Kozlowski	 <krzk@kernel.org>
Subject: Re: [PATCH v7] checkpatch: add uninitialized pointer with __free attribute check
Date: Wed, 03 Dec 2025 08:53:58 -0800	[thread overview]
Message-ID: <4f6a13c78311944f33b8391ea8a915d98be6f792.camel@perches.com> (raw)
In-Reply-To: <20251203-aheev-checkpatch-uninitialized-free-v7-1-841e3b31d8f3@gmail.com>

On Wed, 2025-12-03 at 20:58 +0530, Ally Heev wrote:
> uninitialized pointers with __free attribute can cause undefined
> behavior as the memory randomly assigned 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@linaro.org>
> Link: https://lore.kernel.org/all/8a4c0b43-cf63-400d-b33d-d9c447b7e0b9@suswa.mountain/
> Link: https://lore.kernel.org/all/58fd478f408a34b578ee8d949c5c4b4da4d4f41d.camel@HansenPartnership.com/
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> Signed-off-by: Ally Heev <allyheev@gmail.com>

Acked-by: Joe Perches <joe@perches.com>

> ---
> Testing:
> ran checkpatch.pl before and after the change on 
> crypto/asymmetric_keys/x509_public_key.c, which has
> both initialized with NULL and uninitialized pointers
> ---
> Changes in v7:
> - RESEND. Also, added Reviewed-by trailer
> - Link to v6: https://lore.kernel.org/r/20251125-aheev-checkpatch-uninitialized-free-v6-1-2f3a1d78f678@gmail.com
> 
> Changes in v6:
> - added declaration at the place of use suggestion
> - Link to v5: https://lore.kernel.org/r/20251124-aheev-checkpatch-uninitialized-free-v5-1-0c523b1a3f5a@gmail.com
> 
> Changes in v5:
> - fixed checkpatch doc
> - Link to v4: https://lore.kernel.org/r/20251107-aheev-checkpatch-uninitialized-free-v4-1-4822a6ac728f@gmail.com
> 
> Changes in v4:
> - fixed UNINITIALIZED_PTR_WITH_FREE description 
> - Link to v3: https://lore.kernel.org/r/20251025-aheev-checkpatch-uninitialized-free-v3-1-a67f72b1c2bd@gmail.com
> 
> Changes in v3:
> - remove $FreeAttribute
> - Link to v2: https://lore.kernel.org/r/20251024-aheev-checkpatch-uninitialized-free-v2-0-16c0900e8130@gmail.com
> 
> Changes in v2:
> - change cover letter and title to reflect new changes
> - fix regex to handle multiple declarations in a single line case
> - convert WARN to ERROR for uninitialized pointers
> - add a new WARN for pointers initialized with NULL 
> - NOTE: tried handling multiple declarations on a single line by splitting
>         them and matching the parts with regex, but, it turned out to be 
> 	complex and overkill. Moreover, multi-line declarations pose a threat
> - Link to v1: https://lore.kernel.org/r/20251021-aheev-checkpatch-uninitialized-free-v1-1-18fb01bc6a7a@gmail.com
> ---
>  Documentation/dev-tools/checkpatch.rst | 23 +++++++++++++++++++++++
>  scripts/checkpatch.pl                  |  6 ++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> index d5c47e560324fb2399a5b1bc99c891ed1de10535..b6e02fb91e85710fecfc0a5e5c83a8e7f32d1d3c 100644
> --- a/Documentation/dev-tools/checkpatch.rst
> +++ b/Documentation/dev-tools/checkpatch.rst
> @@ -1009,6 +1009,29 @@ Functions and Variables
>  
>        return bar;
>  
> +  **UNINITIALIZED_PTR_WITH_FREE**
> +    Pointers with __free attribute should be declared at the place of use
> +    and initialized (see include/linux/cleanup.h). In this case
> +    declarations at the top of the function rule can be relaxed. Not doing
> +    so may lead to undefined behavior as the memory assigned (garbage,
> +    in case not initialized) to the pointer is freed automatically when
> +    the pointer goes out of scope.
> +
> +    Also see: https://lore.kernel.org/lkml/58fd478f408a34b578ee8d949c5c4b4da4d4f41d.camel@HansenPartnership.com/
> +
> +    Example::
> +
> +      type var __free(free_func);
> +      ... // var not used, but, in future someone might add a return here
> +      var = malloc(var_size);
> +      ...
> +
> +    should be initialized as::
> +
> +      ...
> +      type var __free(free_func) = malloc(var_size);
> +      ...
> +
>  
>  Permissions
>  -----------
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 92669904eecc7a8d2afd3f2625528e02b6d17cd6..e697d81d71c0b3628f7b59807e8bc40d582621bb 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
> +		while ($line =~ /\*\s*($Ident)\s+__free\s*\(\s*$Ident\s*\)\s*[,;]/g) {
> +			ERROR("UNINITIALIZED_PTR_WITH_FREE",
> +			      "pointer '$1' with __free attribute should be initialized\n" . $herecurr);
> +		}
>  	}
>  
>  	# If we have no input at all, then there is nothing to report on
> 
> ---
> base-commit: 6548d364a3e850326831799d7e3ea2d7bb97ba08
> change-id: 20251021-aheev-checkpatch-uninitialized-free-5c39f75e10a1
> 
> Best regards,

      reply	other threads:[~2025-12-03 16:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-03 15:28 Ally Heev
2025-12-03 16:53 ` Joe Perches [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4f6a13c78311944f33b8391ea8a915d98be6f792.camel@perches.com \
    --to=joe@perches.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=allyheev@gmail.com \
    --cc=apw@canonical.com \
    --cc=corbet@lwn.net \
    --cc=dan.carpenter@linaro.org \
    --cc=dan.j.williams@intel.com \
    --cc=david.hunter.linux@gmail.com \
    --cc=dwaipayanray1@gmail.com \
    --cc=geert@linux-m68k.org \
    --cc=krzk@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=nm@ti.com \
    --cc=sboyd@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=vireshk@kernel.org \
    --cc=workflows@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox