1. Patchew
  2. linux
  3. [PATCH v2 0/2] checkpatch: add checks incorrectly initialized pointers with __free attr
  4. View patch

[PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL

Ally Heev posted 2 patches 3 months, 2 weeks ago
[PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL
Posted by Ally Heev 3 months, 2 weeks ago 
pointers with __free attribute initialized to NULL
pose potential cleanup issues [1] when a function uses
interdependent variables with cleanup attributes

Link: https://docs.kernel.org/core-api/cleanup.html [1]
Link: https://lore.kernel.org/all/68f7b830ec21a_10e910070@dwillia2-mobl4.notmuch/
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Ally Heev <allyheev@gmail.com>
---
 Documentation/dev-tools/checkpatch.rst | 6 ++++++
 scripts/checkpatch.pl                  | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index 1a304bf38bcd27e50bbb7cd4383b07ac54d20b0a..c39213b814f487290d2b0e5d320a4313ada9bbad 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -1015,6 +1015,12 @@ Functions and Variables
     in case not initialized) to the pointer is freed automatically
     when the pointer goes out of scope.
 
+  **NULL_INITIALIZED_PTR_WITH_FREE**
+    Pointers with __free attribute should not be initialized to NULL.
+    Always define and assign such pointers in one statement.
+
+    See: https://docs.kernel.org/core-api/cleanup.html
+
 Permissions
 -----------
 
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1009a4a065e910143dabeee6640b3b3a4bd3fe06..cf186dafc191f1c39d01b3660f19101f6cc61a82 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -7728,6 +7728,12 @@ sub process {
 			ERROR("UNINITIALIZED_PTR_WITH_FREE",
 			      "pointer '$1' with __free attribute should be initialized\n" . $herecurr);
 		}
+
+# check for pointers with __free attribute initialized to NULL
+		while ($line =~ /\*\s*($Ident)\s+$FreeAttribute\s*=\s*NULL\b/g) {
+			WARN("NULL_INITIALIZED_PTR_WITH_FREE",
+			      "pointer '$1' with __free attribute should be initialized to a non-NULL address\n" . $herecurr);
+		}
 	}
 
 	# If we have no input at all, then there is nothing to report on

-- 
2.47.3
Re: [PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL
Posted by ally heev 3 months, 2 weeks ago 
Based on the comments. I will drop this patch in next version
Re: [PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL
Posted by Dan Carpenter 3 months, 2 weeks ago 
On Fri, Oct 24, 2025 at 10:59:16PM +0530, Ally Heev wrote:
> pointers with __free attribute initialized to NULL
> pose potential cleanup issues [1] when a function uses
> interdependent variables with cleanup attributes
> 
> Link: https://docs.kernel.org/core-api/cleanup.html [1]
> Link: https://lore.kernel.org/all/68f7b830ec21a_10e910070@dwillia2-mobl4.notmuch/
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Ally Heev <allyheev@gmail.com>
> ---

I don't think this patch is a good idea...  There are two issues to
consider 1) The absolute number over warnings.  500+ is too high.
2) The ratio of bugs to false positives and we don't have any data on
that but I bet it's low.  It needs to be at least 5%.  For anything
lower than that, you're better off just reviewing code at random
instead of looking through warnings.

regards,
dan carpenter
Re: [PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL
Posted by ally heev 3 months, 2 weeks ago 
On Fri, 2025-10-24 at 21:08 +0300, Dan Carpenter wrote:
> On Fri, Oct 24, 2025 at 10:59:16PM +0530, Ally Heev wrote:
> > pointers with __free attribute initialized to NULL
> > pose potential cleanup issues [1] when a function uses
> > interdependent variables with cleanup attributes
> > 
> > Link: https://docs.kernel.org/core-api/cleanup.html [1]
> > Link: https://lore.kernel.org/all/68f7b830ec21a_10e910070@dwillia2-mobl4.notmuch/
> > Suggested-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Ally Heev <allyheev@gmail.com>
> > ---
> 
> I don't think this patch is a good idea...  There are two issues to
> consider 1) The absolute number over warnings.  500+ is too high.
> 2) The ratio of bugs to false positives and we don't have any data on
> that but I bet it's low.  It needs to be at least 5%.  For anything
> lower than that, you're better off just reviewing code at random
> instead of looking through warnings.
> 
> regards,
> dan carpenter

makes sense

General question about the process for my understanding:
Is checkpatch run on full tree by CI or someone and results reported
regularly ? My understanding was that we would run it only on patches
before submitting them Or we just run it on full tree before adding
new checks to understand if they are catching real issues
Re: [PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL
Posted by Dan Carpenter 3 months, 2 weeks ago 
On Sat, Oct 25, 2025 at 11:53:56AM +0530, ally heev wrote:
> On Fri, 2025-10-24 at 21:08 +0300, Dan Carpenter wrote:
> > On Fri, Oct 24, 2025 at 10:59:16PM +0530, Ally Heev wrote:
> > > pointers with __free attribute initialized to NULL
> > > pose potential cleanup issues [1] when a function uses
> > > interdependent variables with cleanup attributes
> > > 
> > > Link: https://docs.kernel.org/core-api/cleanup.html [1]
> > > Link: https://lore.kernel.org/all/68f7b830ec21a_10e910070@dwillia2-mobl4.notmuch/
> > > Suggested-by: Dan Williams <dan.j.williams@intel.com>
> > > Signed-off-by: Ally Heev <allyheev@gmail.com>
> > > ---
> > 
> > I don't think this patch is a good idea...  There are two issues to
> > consider 1) The absolute number over warnings.  500+ is too high.
> > 2) The ratio of bugs to false positives and we don't have any data on
> > that but I bet it's low.  It needs to be at least 5%.  For anything
> > lower than that, you're better off just reviewing code at random
> > instead of looking through warnings.
> > 
> > regards,
> > dan carpenter
> 
> makes sense
> 
> General question about the process for my understanding:
> Is checkpatch run on full tree by CI or someone and results reported
> regularly ?

Newbies run it regularly.  Otherwise it gets run on subsystem CIs and
the zero-day bot runs it on new patches but it will report the old
warnings as well under the "Old warnings" section.

> My understanding was that we would run it only on patches
> before submitting them Or we just run it on full tree before adding
> new checks to understand if they are catching real issues

Eventually someone will look at all the warnings.  And probably it's
going to be a newbie and so we need to be careful with warning where
newbies might introduce bugs with their changes.

regards,
dan carpenter
Re: [PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL
Posted by ally heev 3 months, 2 weeks ago 
On Mon, Oct 27, 2025 at 10:57 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
> > General question about the process for my understanding:
> > Is checkpatch run on full tree by CI or someone and results reported
> > regularly ?
>
> Newbies run it regularly.  Otherwise it gets run on subsystem CIs and
> the zero-day bot runs it on new patches but it will report the old
> warnings as well under the "Old warnings" section.
>
> > My understanding was that we would run it only on patches
> > before submitting them Or we just run it on full tree before adding
> > new checks to understand if they are catching real issues
>
> Eventually someone will look at all the warnings.  And probably it's
> going to be a newbie and so we need to be careful with warning where
> newbies might introduce bugs with their changes.
>
> regards,
> dan carpenter
>
Makes sense. Thanks!!
---
aheev
Re: [PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL
Posted by Joe Perches 3 months, 2 weeks ago 
On Fri, 2025-10-24 at 22:59 +0530, Ally Heev wrote:
> pointers with __free attribute initialized to NULL
> pose potential cleanup issues [1] when a function uses
> interdependent variables with cleanup attributes
> 
> Link: https://docs.kernel.org/core-api/cleanup.html [1]
> Link: https://lore.kernel.org/all/68f7b830ec21a_10e910070@dwillia2-mobl4.notmuch/
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Ally Heev <allyheev@gmail.com>
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -7728,6 +7728,12 @@ sub process {
>  			ERROR("UNINITIALIZED_PTR_WITH_FREE",
>  			      "pointer '$1' with __free attribute should be initialized\n" . $herecurr);
>  		}
> +
> +# check for pointers with __free attribute initialized to NULL
> +		while ($line =~ /\*\s*($Ident)\s+$FreeAttribute\s*=\s*NULL\b/g) {
> +			WARN("NULL_INITIALIZED_PTR_WITH_FREE",
> +			      "pointer '$1' with __free attribute should be initialized to a non-NULL address\n" . $herecurr);
> +		}
>  	}

I think this a poor idea as almost all the instances where this
initialization is done are fine.

And there are a lot of them.

$ git grep -P '\b__free\b.*=\s*NULL\s*;' | wc -l
490

And what about these uses that depend on struct path members
.mnt and .dentry being NULL. 

$ git grep -P '\b__free\b.*=\s*\{.*\}\s*;'
fs/configfs/symlink.c:  struct path path __free(path_put) = {};
fs/fhandle.c:   struct path path __free(path_put) = {};
fs/file_attr.c: struct path filepath __free(path_put) = {};
fs/file_attr.c: struct path filepath __free(path_put) = {};
fs/namei.c:     struct path parent_path __free(path_put) = {};
fs/namei.c:     struct path parent_path __free(path_put) = {};
fs/namespace.c: struct path old_path __free(path_put) = {};
fs/namespace.c: struct path path __free(path_put) = {};
fs/namespace.c: struct path old_path __free(path_put) = {};
fs/namespace.c: struct path path __free(path_put) = {};
fs/namespace.c: struct path to_path __free(path_put) = {};
fs/namespace.c: struct path from_path __free(path_put) = {};
fs/namespace.c: struct path new __free(path_put) = {};
fs/namespace.c: struct path old __free(path_put) = {};
fs/namespace.c: struct path root __free(path_put) = {};
fs/namespace.c: struct klistmount kls __free(klistmount_free) = {};
fs/namespace.c: struct path fs_root __free(path_put) = {};
fs/nsfs.c:      struct path path __free(path_put) = {};
fs/nsfs.c:              struct path path __free(path_put) = {};
fs/nsfs.c:      struct path path __free(path_put) = {};
fs/overlayfs/params.c:  struct path layer_path __free(path_put) = {};
fs/overlayfs/params.c:          struct path path __free(path_put) = {};
fs/pidfs.c:     struct path path __free(path_put) = {};
include/linux/path.h: * struct path path __free(path_put) = {};
kernel/acct.c:  struct path internal __free(path_put) = {};     // in that order
kernel/trace/trace_uprobe.c:    struct path path __free(path_put) = {};
Re: [PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL
Posted by ally heev 3 months, 2 weeks ago 
On Fri, 2025-10-24 at 11:01 -0700, Joe Perches wrote:
[..]
> > @@ -7728,6 +7728,12 @@ sub process {
> >  			ERROR("UNINITIALIZED_PTR_WITH_FREE",
> >  			      "pointer '$1' with __free attribute should be initialized\n" . $herecurr);
> >  		}
> > +
> > +# check for pointers with __free attribute initialized to NULL
> > +		while ($line =~ /\*\s*($Ident)\s+$FreeAttribute\s*=\s*NULL\b/g) {
> > +			WARN("NULL_INITIALIZED_PTR_WITH_FREE",
> > +			      "pointer '$1' with __free attribute should be initialized to a non-NULL address\n" . $herecurr);
> > +		}
> >  	}
> 
> I think this a poor idea as almost all the instances where this
> initialization is done are fine.
> 
> And there are a lot of them.
> 
> $ git grep -P '\b__free\b.*=\s*NULL\s*;' | wc -l
> 490

Sorry for not checking this earlier. I looked at quite a few of them
none were real issues

> 
> And what about these uses that depend on struct path members
> .mnt and .dentry being NULL. 
> 
> $ git grep -P '\b__free\b.*=\s*\{.*\}\s*;'
> fs/configfs/symlink.c:  struct path path __free(path_put) = {};
> fs/fhandle.c:   struct path path __free(path_put) = {};
> fs/file_attr.c: struct path filepath __free(path_put) = {};
> fs/file_attr.c: struct path filepath __free(path_put) = {};
> fs/namei.c:     struct path parent_path __free(path_put) = {};
> fs/namei.c:     struct path parent_path __free(path_put) = {};
> fs/namespace.c: struct path old_path __free(path_put) = {};
> fs/namespace.c: struct path path __free(path_put) = {};
> fs/namespace.c: struct path old_path __free(path_put) = {};
> fs/namespace.c: struct path path __free(path_put) = {};
> fs/namespace.c: struct path to_path __free(path_put) = {};
> fs/namespace.c: struct path from_path __free(path_put) = {};
> fs/namespace.c: struct path new __free(path_put) = {};
> fs/namespace.c: struct path old __free(path_put) = {};
> fs/namespace.c: struct path root __free(path_put) = {};
> fs/namespace.c: struct klistmount kls __free(klistmount_free) = {};
> fs/namespace.c: struct path fs_root __free(path_put) = {};
> fs/nsfs.c:      struct path path __free(path_put) = {};
> fs/nsfs.c:              struct path path __free(path_put) = {};
> fs/nsfs.c:      struct path path __free(path_put) = {};
> fs/overlayfs/params.c:  struct path layer_path __free(path_put) = {};
> fs/overlayfs/params.c:          struct path path __free(path_put) = {};
> fs/pidfs.c:     struct path path __free(path_put) = {};
> include/linux/path.h: * struct path path __free(path_put) = {};
> kernel/acct.c:  struct path internal __free(path_put) = {};     // in that order
> kernel/trace/trace_uprobe.c:    struct path path __free(path_put) = {};

These are not valid issues too
Re: [PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL
Posted by dan.j.williams@intel.com 3 months, 2 weeks ago 
Joe Perches wrote:
> On Fri, 2025-10-24 at 22:59 +0530, Ally Heev wrote:
> > pointers with __free attribute initialized to NULL
> > pose potential cleanup issues [1] when a function uses
> > interdependent variables with cleanup attributes
> > 
> > Link: https://docs.kernel.org/core-api/cleanup.html [1]
> > Link: https://lore.kernel.org/all/68f7b830ec21a_10e910070@dwillia2-mobl4.notmuch/
> > Suggested-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Ally Heev <allyheev@gmail.com>
> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -7728,6 +7728,12 @@ sub process {
> >  			ERROR("UNINITIALIZED_PTR_WITH_FREE",
> >  			      "pointer '$1' with __free attribute should be initialized\n" . $herecurr);
> >  		}
> > +
> > +# check for pointers with __free attribute initialized to NULL
> > +		while ($line =~ /\*\s*($Ident)\s+$FreeAttribute\s*=\s*NULL\b/g) {
> > +			WARN("NULL_INITIALIZED_PTR_WITH_FREE",
> > +			      "pointer '$1' with __free attribute should be initialized to a non-NULL address\n" . $herecurr);
> > +		}
> >  	}
> 
> I think this a poor idea as almost all the instances where this
> initialization is done are fine.
> 
> And there are a lot of them.
> 
> $ git grep -P '\b__free\b.*=\s*NULL\s*;' | wc -l
> 490

That is significant. ...but you did say "almost" above. What about
moving this from WARN level to CHK level?

With that change you can add:

Acked-by: Dan Williams <dan.j.williams@intel.com>
Re: [PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL
Posted by Joe Perches 3 months, 2 weeks ago 
On Fri, 2025-10-24 at 11:14 -0700, dan.j.williams@intel.com wrote:
> Joe Perches wrote:
> > On Fri, 2025-10-24 at 22:59 +0530, Ally Heev wrote:
> > > pointers with __free attribute initialized to NULL
> > > pose potential cleanup issues [1] when a function uses
> > > interdependent variables with cleanup attributes
> > > 
> > > Link: https://docs.kernel.org/core-api/cleanup.html [1]
> > > Link: https://lore.kernel.org/all/68f7b830ec21a_10e910070@dwillia2-mobl4.notmuch/
> > > Suggested-by: Dan Williams <dan.j.williams@intel.com>
> > > Signed-off-by: Ally Heev <allyheev@gmail.com>
> > []
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > @@ -7728,6 +7728,12 @@ sub process {
> > >  			ERROR("UNINITIALIZED_PTR_WITH_FREE",
> > >  			      "pointer '$1' with __free attribute should be initialized\n" . $herecurr);
> > >  		}
> > > +
> > > +# check for pointers with __free attribute initialized to NULL
> > > +		while ($line =~ /\*\s*($Ident)\s+$FreeAttribute\s*=\s*NULL\b/g) {
> > > +			WARN("NULL_INITIALIZED_PTR_WITH_FREE",
> > > +			      "pointer '$1' with __free attribute should be initialized to a non-NULL address\n" . $herecurr);
> > > +		}
> > >  	}
> > 
> > I think this a poor idea as almost all the instances where this
> > initialization is done are fine.
> > 
> > And there are a lot of them.
> > 
> > $ git grep -P '\b__free\b.*=\s*NULL\s*;' | wc -l
> > 490
> 
> That is significant. ...but you did say "almost" above. What about
> moving this from WARN level to CHK level?

I have no idea how many instances in the tree are inappropriate.
Do you? I believe it to be a difficult analysis problem.

But given the number is likely to be extremely low, I think it should
not be added to checkpatch even as a CHK.

If you can show that the reporting rate of defects is significant,
say >10%, then OK, but I rather doubt it's that high.
Re: [PATCH v2 2/2] add check for pointers with __free attribute initialized to NULL
Posted by dan.j.williams@intel.com 3 months, 2 weeks ago 
Joe Perches wrote:
[..]
> > > And there are a lot of them.
> > > 
> > > $ git grep -P '\b__free\b.*=\s*NULL\s*;' | wc -l
> > > 490
> > 
> > That is significant. ...but you did say "almost" above. What about
> > moving this from WARN level to CHK level?
> 
> I have no idea how many instances in the tree are inappropriate.
> Do you? I believe it to be a difficult analysis problem.
> 
> But given the number is likely to be extremely low, I think it should
> not be added to checkpatch even as a CHK.
> 
> If you can show that the reporting rate of defects is significant,
> say >10%, then OK, but I rather doubt it's that high.

Fair enough. Ally, thanks for taking a look.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.