[PATCH v2 0/2] checkpatch: add checks incorrectly initialized pointers with __free attr

[PATCH v2 0/2] checkpatch: add checks incorrectly initialized pointers with __free attr

[Ally Heev]

Badly initialized pointers with __free attr can
cause cleanup issues. So,

Adding checks for
- uninitialized pointers
- initialized pointers with NULL

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 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

---
Ally Heev (2):
      checkpatch: add uninitialized pointer with __free attribute check
      add check for pointers with __free attribute initialized to NULL

 Documentation/dev-tools/checkpatch.rst | 11 +++++++++++
 scripts/checkpatch.pl                  | 13 +++++++++++++
 2 files changed, 24 insertions(+)
---
base-commit: 6548d364a3e850326831799d7e3ea2d7bb97ba08
change-id: 20251021-aheev-checkpatch-uninitialized-free-5c39f75e10a1

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

[PATCH v2 1/2] checkpatch: add uninitialized pointer with __free attribute check

[Ally Heev]

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@linaro.org>
Link: https://lore.kernel.org/all/8a4c0b43-cf63-400d-b33d-d9c447b7e0b9@suswa.mountain/
Acked-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Ally Heev <allyheev@gmail.com>
---
 Documentation/dev-tools/checkpatch.rst | 5 +++++
 scripts/checkpatch.pl                  | 7 +++++++
 2 files changed, 12 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..1009a4a065e910143dabeee6640b3b3a4bd3fe06 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -509,6 +509,7 @@ our $InitAttributeData = qr{$InitAttributePrefix(?:initdata\b)};
 our $InitAttributeConst = qr{$InitAttributePrefix(?:initconst\b)};
 our $InitAttributeInit = qr{$InitAttributePrefix(?:init\b)};
 our $InitAttribute = qr{$InitAttributeData|$InitAttributeConst|$InitAttributeInit};
+our $FreeAttribute = qr{__free\s*\(\s*$Ident\s*\)};
 
 # Notes to $Attribute:
 # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check
@@ -7721,6 +7722,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+$FreeAttribute\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

-- 
2.47.3

Re: [PATCH v2 1/2] checkpatch: add uninitialized pointer with __free attribute check

[Joe Perches]

On Fri, 2025-10-24 at 22:59 +0530, 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@linaro.org>
> Link: https://lore.kernel.org/all/8a4c0b43-cf63-400d-b33d-d9c447b7e0b9@suswa.mountain/
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Ally Heev <allyheev@gmail.com>
> ---
>  Documentation/dev-tools/checkpatch.rst | 5 +++++
>  scripts/checkpatch.pl                  | 7 +++++++
>  2 files changed, 12 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..1009a4a065e910143dabeee6640b3b3a4bd3fe06 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -509,6 +509,7 @@ our $InitAttributeData = qr{$InitAttributePrefix(?:initdata\b)};
>  our $InitAttributeConst = qr{$InitAttributePrefix(?:initconst\b)};
>  our $InitAttributeInit = qr{$InitAttributePrefix(?:init\b)};
>  our $InitAttribute = qr{$InitAttributeData|$InitAttributeConst|$InitAttributeInit};
> +our $FreeAttribute = qr{__free\s*\(\s*$Ident\s*\)};

If you are really suggesting using this, and I don't think it's
particularly useful, please use

out $InitAttributeFree = qr{$InitAttributePrefix(?:free\s*\(\s*$Ident\s*\)};

>  
>  # Notes to $Attribute:
>  # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check
> @@ -7721,6 +7722,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+$FreeAttribute\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

Re: [PATCH v2 1/2] checkpatch: add uninitialized pointer with __free attribute check

[ally heev]

On Fri, 2025-10-24 at 11:14 -0700, Joe Perches wrote:
> On Fri, 2025-10-24 at 22:59 +0530, 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@linaro.org>
> > Link: https://lore.kernel.org/all/8a4c0b43-cf63-400d-b33d-d9c447b7e0b9@suswa.mountain/
> > Acked-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Ally Heev <allyheev@gmail.com>
> > ---
> >  Documentation/dev-tools/checkpatch.rst | 5 +++++
> >  scripts/checkpatch.pl                  | 7 +++++++
> >  2 files changed, 12 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..1009a4a065e910143dabeee6640b3b3a4bd3fe06 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -509,6 +509,7 @@ our $InitAttributeData = qr{$InitAttributePrefix(?:initdata\b)};
> >  our $InitAttributeConst = qr{$InitAttributePrefix(?:initconst\b)};
> >  our $InitAttributeInit = qr{$InitAttributePrefix(?:init\b)};
> >  our $InitAttribute = qr{$InitAttributeData|$InitAttributeConst|$InitAttributeInit};
> > +our $FreeAttribute = qr{__free\s*\(\s*$Ident\s*\)};
> 
> If you are really suggesting using this, and I don't think it's
> particularly useful, please use
> 
> out $InitAttributeFree = qr{$InitAttributePrefix(?:free\s*\(\s*$Ident\s*\)};

Thanks, I'll check it out

> >  
> >  # Notes to $Attribute:
> >  # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check
> > @@ -7721,6 +7722,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+$FreeAttribute\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

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

[Ally Heev]

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

[Joe Perches]

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

[dan.j.williams]

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

[Joe Perches]

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

[dan.j.williams]

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.

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

[ally heev]

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

[Dan Carpenter]

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

[ally heev]

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

[Dan Carpenter]

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

[ally heev]

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

[ally heev]

Based on the comments. I will drop this patch in next version