Maintainer workflows discussions
 help / color / mirror / Atom feed
* Re: [PATCH] coding-style: fix verb typo
From: Jonathan Corbet @ 2025-11-03 23:11 UTC (permalink / raw)
  To: Gabriele Ricciardi; +Cc: workflows, linux-doc, linux-kernel, Gabriele Ricciardi
In-Reply-To: <20251101223027.171874-1-gricciardi-coding@pm.me>

Gabriele Ricciardi <gricciardi-coding@pm.me> writes:

> In the Identation section there is a list of instructions in
> second-person. The offending line uses third-person singular.
>
> Signed-off-by: Gabriele Ricciardi <gricciardi-coding@pm.me>
> ---
>  Documentation/process/coding-style.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> index d1a8e5465ed9..2969ca378dbb 100644
> --- a/Documentation/process/coding-style.rst
> +++ b/Documentation/process/coding-style.rst
> @@ -76,7 +76,7 @@ Don't use commas to avoid using braces:
>  	if (condition)
>  		do_this(), do_that();
>  
> -Always uses braces for multiple statements:
> +Always use braces for multiple statements:
>  
>  .. code-block:: c

Applied, thanks.

jon

^ permalink raw reply

* [PATCH RESEND v3] checkpatch: add uninitialized pointer with __free attribute check
From: Ally Heev @ 2025-11-04  9:56 UTC (permalink / raw)
  To: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
	Andy Whitcroft
  Cc: workflows, linux-doc, linux-kernel, Dan Carpenter, David Hunter,
	Shuah Khan, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm,
	dan.j.williams, 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>
---
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 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 | 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..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,
-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQQBFRpOLrIakF7DYvaWPaLUP9d7HAUCaPyCqQAKCRCWPaLUP9d7
HMY1AP93A0fWIkV06Vcd8EHJy3w2G8hoinjDBpy5dZIXMMQFJgEA945Vs2tysbbR
qVNPU2EM8cnDwRabEkET597he3AbpgM=
=PL1b
-----END PGP SIGNATURE-----
-- 
Ally Heev <allyheev@gmail.com>


^ permalink raw reply related

* Re: [PATCH RESEND v3] checkpatch: add uninitialized pointer with __free attribute check
From: Geert Uytterhoeven @ 2025-11-04 13:28 UTC (permalink / raw)
  To: Ally Heev
  Cc: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
	Andy Whitcroft, workflows, linux-doc, linux-kernel, Dan Carpenter,
	David Hunter, Shuah Khan, Viresh Kumar, Nishanth Menon,
	Stephen Boyd, linux-pm, dan.j.williams
In-Reply-To: <20251104-aheev-checkpatch-uninitialized-free-v3-1-d94ccef4917a@gmail.com>

Hi Ally,

On Tue, 4 Nov 2025 at 10:58, Ally Heev <allyheev@gmail.com> 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>

Thanks for your patch!

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

I think this is misleading, and can be improved: if the pointer is
uninitialized, no memory was allocated?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH RESEND v3] checkpatch: add uninitialized pointer with __free attribute check
From: ally heev @ 2025-11-05  6:27 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
	Andy Whitcroft, workflows, linux-doc, linux-kernel, Dan Carpenter,
	David Hunter, Shuah Khan, Viresh Kumar, Nishanth Menon,
	Stephen Boyd, linux-pm, dan.j.williams
In-Reply-To: <CAMuHMdV+12MoAGNHC9kf==Bt0cLuJ39Fs+W61DN67sE_p-u=og@mail.gmail.com>

On Tue, 2025-11-04 at 14:28 +0100, Geert Uytterhoeven wrote:
> Hi Ally,
> 
> On Tue, 4 Nov 2025 at 10:58, Ally Heev <allyheev@gmail.com> 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>
> 
> Thanks for your patch!
> 
> > --- 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.
> 
> I think this is misleading, and can be improved: if the pointer is
> uninitialized, no memory was allocated?

yeah right. Will update in next version

Regards,
Ally


^ permalink raw reply

* Re: [PATCH RESEND v3] checkpatch: add uninitialized pointer with __free attribute check
From: Markus Elfring @ 2025-11-05  9:18 UTC (permalink / raw)
  To: Ally Heev, Geert Uytterhoeven, linux-doc, workflows, Joe Perches
  Cc: LKML, linux-pm, Andy Whitcroft, Jonathan Corbet, Dan Carpenter,
	Dan Williams, David Hunter, Dwaipayan Ray, Lukas Bulwahn,
	Nishanth Menon, Shuah Khan, Stephen Boyd, Viresh Kumar
In-Reply-To: <CAMuHMdV+12MoAGNHC9kf==Bt0cLuJ39Fs+W61DN67sE_p-u=og@mail.gmail.com>

…
> > +++ 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.
> 
> I think this is misleading, and can be improved: if the pointer is
> uninitialized, no memory was allocated?

* Do corresponding source code analysis requirements indicate a need
  to perform data processing with other programming interfaces than regular expressions?

* How do you think about to mention the possibility once more that scopes
  can be reduced for affected local variables?
  https://elixir.bootlin.com/linux/v6.18-rc4/source/include/linux/cleanup.h#L142-L146


Regards,
Markus

^ permalink raw reply

* Re: [PATCH RESEND v3] checkpatch: add uninitialized pointer with __free attribute check
From: ally heev @ 2025-11-06 16:29 UTC (permalink / raw)
  To: Markus Elfring, Geert Uytterhoeven, linux-doc, workflows,
	Joe Perches
  Cc: LKML, linux-pm, Andy Whitcroft, Jonathan Corbet, Dan Carpenter,
	Dan Williams, David Hunter, Dwaipayan Ray, Lukas Bulwahn,
	Nishanth Menon, Shuah Khan, Stephen Boyd, Viresh Kumar
In-Reply-To: <221c2b9b-4809-48d8-af7d-f290d1c2a7fa@web.de>

On Wed, 2025-11-05 at 10:18 +0100, Markus Elfring wrote:
[..]
> * Do corresponding source code analysis requirements indicate a need
>   to perform data processing with other programming interfaces than regular expressions?
> 

not sure about other source code analysis tools, but checkpatch
predominantly uses regexes

> * How do you think about to mention the possibility once more that scopes
>   can be reduced for affected local variables?
>   https://elixir.bootlin.com/linux/v6.18-rc4/source/include/linux/cleanup.h#L142-L146
>  ...

The docstring talks about interdependency issues caused by assigning to
`NULL` which are very rare

Regards,
Ally



^ permalink raw reply

* Re: [v3] checkpatch: add uninitialized pointer with __free attribute check
From: Markus Elfring @ 2025-11-06 17:03 UTC (permalink / raw)
  To: Ally Heev, Geert Uytterhoeven, linux-doc, workflows, Joe Perches
  Cc: LKML, linux-pm, Andy Whitcroft, Jonathan Corbet, Dan Carpenter,
	Dan Williams, David Hunter, Dwaipayan Ray, Lukas Bulwahn,
	Nishanth Menon, Shuah Khan, Stephen Boyd, Viresh Kumar
In-Reply-To: <67e28fadf4c20433c964d13d96dafe3514457656.camel@gmail.com>

>> * Do corresponding source code analysis requirements indicate a need
>>   to perform data processing with other programming interfaces than regular expressions?
> 
> not sure about other source code analysis tools, but checkpatch
> predominantly uses regexes

Would you become interested to discuss additional software design approaches?


>> * How do you think about to mention the possibility once more that scopes
>>   can be reduced for affected local variables?
>>   https://elixir.bootlin.com/linux/v6.18-rc4/source/include/linux/cleanup.h#L142-L146
>>  ...
> 
> The docstring talks about interdependency issues caused by assigning to
> `NULL` which are very rare

It seems that you interpret a linked information source in a special direction.
Several developers are stumbling on challenges also according to software evolution.

Regards,
Markus

^ permalink raw reply

* [PATCH v4] checkpatch: add uninitialized pointer with __free attribute check
From: Ally Heev @ 2025-11-07  6:56 UTC (permalink / raw)
  To: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
	Andy Whitcroft
  Cc: workflows, linux-doc, linux-kernel, Dan Carpenter, David Hunter,
	Shuah Khan, Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm,
	dan.j.williams, Geert Uytterhoeven, Ally Heev

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/
Acked-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Ally Heev <allyheev@gmail.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 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 | 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..c61a3892a60c13f7c5ba89e969e39a93a3dcd5bc 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 assigned (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..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,
-- 
Ally Heev <allyheev@gmail.com>


^ permalink raw reply related

* Re: [PATCH] [v2] Documentation: Provide guidelines for tool-generated content
From: Vlastimil Babka @ 2025-11-10  7:43 UTC (permalink / raw)
  To: Dave Hansen, linux-kernel, workflows@vger.kernel.org,
	ksummit@lists.linux.dev
  Cc: Steven Rostedt, Dan Williams, Theodore Ts'o, Sasha Levin,
	Jonathan Corbet, Kees Cook, Greg Kroah-Hartman, Miguel Ojeda,
	Shuah Khan
In-Reply-To: <20251105231514.3167738-1-dave.hansen@linux.intel.com>

+Cc ksummit (where the discussions about this topic happened recently) and
workflows (probably the closest list we have for such things in general)
because nobody reads lkml today and this seems to have been going under the
radar until mentioned at lwn yesterday

On 11/6/25 00:15, Dave Hansen wrote:
> In the last few years, the capabilities of coding tools have exploded.
> As those capabilities have expanded, contributors and maintainers have
> more and more questions about how and when to apply those
> capabilities.
> 
> The shiny new AI tools (chatbots, coding assistants and more) are
> impressive.  Add new Documentation to guide contributors on how to
> best use kernel development tools, new and old.
> 
> Note, though, there are fundamentally no new or unique rules in this
> new document. It clarifies expectations that the kernel community has
> had for many years. For example, researchers are already asked to
> disclose the tools they use to find issues in
> Documentation/process/researcher-guidelines.rst. This new document
> just reiterates existing best practices for development tooling.
> 
> In short: Please show your work and make sure your contribution is
> easy to review.
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Theodore Ts'o <tytso@mit.edu>
> Cc: Sasha Levin <sashal@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Kees Cook <kees@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> 
> --
> 
> This document was a collaborative effort from all the members of
> the TAB. I just reformatted it into .rst and wrote the changelog.
> 
> Changes from v1:
>  * Rename to generated-content.rst and add to documentation index.
>    (Jon)
>  * Rework subject to align with the new filename
>  * Replace commercial names with generic ones. (Jon)
>  * Be consistent about punctuation at the end of bullets for whole
>    sentences. (Miguel)
>  * Formatting sprucing up and minor typos (Miguel)
> ---
>  Documentation/process/generated-content.rst | 94 +++++++++++++++++++++
>  Documentation/process/index.rst             |  1 +
>  2 files changed, 95 insertions(+)
>  create mode 100644 Documentation/process/generated-content.rst
> 
> diff --git a/Documentation/process/generated-content.rst b/Documentation/process/generated-content.rst
> new file mode 100644
> index 0000000000000..5e8ff44190932
> --- /dev/null
> +++ b/Documentation/process/generated-content.rst
> @@ -0,0 +1,94 @@
> +============================================
> +Kernel Guidelines for Tool Generated Content
> +============================================
> +
> +Purpose
> +=======
> +
> +Kernel contributors have been using tooling to generate contributions
> +for a long time. These tools are constantly becoming more capable and
> +undoubtedly improve developer productivity. At the same time, reviewer
> +and maintainer bandwidth is a very scarce resource. Understanding
> +which portions of a contribution come from humans versus tools is
> +critical to maintain those resources and keep kernel development
> +healthy.
> +
> +The goal here is to clarify community expectations around tools. This
> +lets everyone become more productive while also maintaining high
> +degrees of trust between submitters and reviewers.
> +
> +Out of Scope
> +============
> +
> +These guidelines do not apply to tools that make trivial tweaks to
> +preexisting content. Nor do they pertain to AI tooling that helps with
> +menial tasks. Some examples:
> +
> + - Spelling and grammar fix ups, like rephrasing to imperative voice
> + - Typing aids like identifier completion, common boilerplate or
> +   trivial pattern completion
> + - Purely mechanical transformations like variable renaming
> + - Reformatting, like running Lindent, ``clang-format`` or
> +   ``rust-fmt``
> +
> +Even if your tool use is out of scope you should still always consider
> +if it would help reviewing your contribution if the reviewer knows
> +about the tool that you used.
> +
> +In Scope
> +========
> +
> +These guidelines apply when a meaningful amount of content in a kernel
> +contribution was not written by a person in the Signed-off-by chain,
> +but was instead created by a tool.
> +
> +Detection of a problem is also a part of the development process; if a
> +tool was used to find a problem addressed by a change, that should be
> +noted in the changelog. This not only gives credit where it is due, it
> +also helps fellow developers find out about these tools.
> +
> +Some examples:
> + - Any tool-suggested fix such as ``checkpatch.pl --fix``
> + - Coccinelle scripts
> + - A chatbot generated a new function in your patch to sort list entries.
> + - A .c file in the patch was originally generated by a LLM but cleaned
> +   up by hand.
> + - The changelog was generated by handing the patch to a generative AI
> +   tool and asking it to write the changelog.
> + - The changelog was translated from another language.
> +
> +If in doubt, choose transparency and assume these guidelines apply to
> +your contribution.
> +
> +Guidelines
> +==========
> +
> +First, read the Developer's Certificate of Origin:
> +Documentation/process/submitting-patches.rst . Its rules are simple
> +and have been in place for a long time. They have covered many
> +tool-generated contributions.
> +
> +Second, when making a contribution, be transparent about the origin of
> +content in cover letters and changelogs. You can be more transparent
> +by adding information like this:
> +
> + - What tools were used?
> + - The input to the tools you used, like the coccinelle source script.
> + - If code was largely generated from a single or short set of
> +   prompts, include those prompts in the commit log. For longer
> +   sessions, include a summary of the prompts and the nature of
> +   resulting assistance.
> + - Which portions of the content were affected by that tool?
> +
> +As with all contributions, individual maintainers have discretion to
> +choose how they handle the contribution. For example, they might:
> +
> + - Treat it just like any other contribution
> + - Reject it outright
> + - Review the contribution with extra scrutiny
> + - Suggest a better prompt instead of suggesting specific code changes
> + - Ask for some other special steps, like asking the contributor to
> +   elaborate on how the tool or model was trained
> + - Ask the submitter to explain in more detail about the contribution
> +   so that the maintainer can feel comfortable that the submitter fully
> +   understands how the code works.
> diff --git a/Documentation/process/index.rst b/Documentation/process/index.rst
> index aa12f26601949..e1a8a31389f53 100644
> --- a/Documentation/process/index.rst
> +++ b/Documentation/process/index.rst
> @@ -68,6 +68,7 @@ beyond).
>     stable-kernel-rules
>     management-style
>     researcher-guidelines
> +   generated-content
>  
>  Dealing with bugs
>  -----------------


^ permalink raw reply

* Re: [PATCH] [v2] Documentation: Provide guidelines for tool-generated content
From: Christian Brauner @ 2025-11-10  8:58 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Vlastimil Babka, linux-kernel, workflows@vger.kernel.org,
	ksummit@lists.linux.dev, Steven Rostedt, Dan Williams,
	Theodore Ts'o, Sasha Levin, Jonathan Corbet, Kees Cook,
	Greg Kroah-Hartman, Miguel Ojeda, Shuah Khan
In-Reply-To: <653b4187-ec4f-4f5d-ae76-d37f46070cb4@suse.cz>

On Mon, Nov 10, 2025 at 08:43:06AM +0100, Vlastimil Babka wrote:
> +Cc ksummit (where the discussions about this topic happened recently) and
> workflows (probably the closest list we have for such things in general)
> because nobody reads lkml today and this seems to have been going under the
> radar until mentioned at lwn yesterday
> 
> On 11/6/25 00:15, Dave Hansen wrote:
> > In the last few years, the capabilities of coding tools have exploded.
> > As those capabilities have expanded, contributors and maintainers have
> > more and more questions about how and when to apply those
> > capabilities.
> > 
> > The shiny new AI tools (chatbots, coding assistants and more) are
> > impressive.

This reads like a factual statement about "impressiveness" of the tools.
Just drop that sentence, please. It doesn't add value to the commit
message at all.
                  
> > Add new Documentation to guide contributors on how to 
> > best use kernel development tools, new and old.
> > 
> > Note, though, there are fundamentally no new or unique rules in this
> > new document. It clarifies expectations that the kernel community has
> > had for many years. For example, researchers are already asked to
> > disclose the tools they use to find issues in
> > Documentation/process/researcher-guidelines.rst. This new document
> > just reiterates existing best practices for development tooling.
> > 
> > In short: Please show your work and make sure your contribution is
> > easy to review.
> > 
> > Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: Theodore Ts'o <tytso@mit.edu>
> > Cc: Sasha Levin <sashal@kernel.org>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Kees Cook <kees@kernel.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Miguel Ojeda <ojeda@kernel.org>
> > Cc: Shuah Khan <shuah@kernel.org>
> > 
> > --
> > 
> > This document was a collaborative effort from all the members of
> > the TAB. I just reformatted it into .rst and wrote the changelog.
> > 
> > Changes from v1:
> >  * Rename to generated-content.rst and add to documentation index.
> >    (Jon)
> >  * Rework subject to align with the new filename
> >  * Replace commercial names with generic ones. (Jon)
> >  * Be consistent about punctuation at the end of bullets for whole
> >    sentences. (Miguel)
> >  * Formatting sprucing up and minor typos (Miguel)
> > ---
> >  Documentation/process/generated-content.rst | 94 +++++++++++++++++++++
> >  Documentation/process/index.rst             |  1 +
> >  2 files changed, 95 insertions(+)
> >  create mode 100644 Documentation/process/generated-content.rst
> > 
> > diff --git a/Documentation/process/generated-content.rst b/Documentation/process/generated-content.rst
> > new file mode 100644
> > index 0000000000000..5e8ff44190932
> > --- /dev/null
> > +++ b/Documentation/process/generated-content.rst
> > @@ -0,0 +1,94 @@
> > +============================================
> > +Kernel Guidelines for Tool Generated Content
> > +============================================
> > +
> > +Purpose
> > +=======
> > +
> > +Kernel contributors have been using tooling to generate contributions
> > +for a long time.

> > These tools are constantly becoming more capable and
> > +undoubtedly improve developer productivity. At the same time, reviewer

"undoubtedly improve developer productivity"?
Am I reading an advert or kernel documentation about the policy how to
use new tooling?

Please keep it factual without statements about what perceived value
this adds. People use it and we have to have a policy for it. There's no
need to celebrate it.

> > +and maintainer bandwidth is a very scarce resource. Understanding
> > +which portions of a contribution come from humans versus tools is
> > +critical to maintain those resources and keep kernel development
> > +healthy.
> > +
> > +The goal here is to clarify community expectations around tools. This
> > +lets everyone become more productive while also maintaining high
> > +degrees of trust between submitters and reviewers.
> > +
> > +Out of Scope
> > +============
> > +
> > +These guidelines do not apply to tools that make trivial tweaks to
> > +preexisting content. Nor do they pertain to AI tooling that helps with
> > +menial tasks. Some examples:
> > +
> > + - Spelling and grammar fix ups, like rephrasing to imperative voice
> > + - Typing aids like identifier completion, common boilerplate or
> > +   trivial pattern completion
> > + - Purely mechanical transformations like variable renaming
> > + - Reformatting, like running Lindent, ``clang-format`` or
> > +   ``rust-fmt``
> > +
> > +Even if your tool use is out of scope you should still always consider
> > +if it would help reviewing your contribution if the reviewer knows
> > +about the tool that you used.
> > +
> > +In Scope
> > +========
> > +
> > +These guidelines apply when a meaningful amount of content in a kernel
> > +contribution was not written by a person in the Signed-off-by chain,
> > +but was instead created by a tool.
> > +
> > +Detection of a problem is also a part of the development process; if a
> > +tool was used to find a problem addressed by a change, that should be
> > +noted in the changelog. This not only gives credit where it is due, it
> > +also helps fellow developers find out about these tools.
> > +
> > +Some examples:
> > + - Any tool-suggested fix such as ``checkpatch.pl --fix``
> > + - Coccinelle scripts
> > + - A chatbot generated a new function in your patch to sort list entries.
> > + - A .c file in the patch was originally generated by a LLM but cleaned
> > +   up by hand.
> > + - The changelog was generated by handing the patch to a generative AI
> > +   tool and asking it to write the changelog.
> > + - The changelog was translated from another language.
> > +
> > +If in doubt, choose transparency and assume these guidelines apply to
> > +your contribution.
> > +
> > +Guidelines
> > +==========
> > +
> > +First, read the Developer's Certificate of Origin:
> > +Documentation/process/submitting-patches.rst . Its rules are simple
> > +and have been in place for a long time. They have covered many
> > +tool-generated contributions.
> > +
> > +Second, when making a contribution, be transparent about the origin of
> > +content in cover letters and changelogs. You can be more transparent
> > +by adding information like this:
> > +
> > + - What tools were used?
> > + - The input to the tools you used, like the coccinelle source script.
> > + - If code was largely generated from a single or short set of
> > +   prompts, include those prompts in the commit log. For longer
> > +   sessions, include a summary of the prompts and the nature of
> > +   resulting assistance.
> > + - Which portions of the content were affected by that tool?
> > +
> > +As with all contributions, individual maintainers have discretion to
> > +choose how they handle the contribution. For example, they might:
> > +
> > + - Treat it just like any other contribution
> > + - Reject it outright
> > + - Review the contribution with extra scrutiny
> > + - Suggest a better prompt instead of suggesting specific code changes
> > + - Ask for some other special steps, like asking the contributor to
> > +   elaborate on how the tool or model was trained
> > + - Ask the submitter to explain in more detail about the contribution
> > +   so that the maintainer can feel comfortable that the submitter fully
> > +   understands how the code works.
> > diff --git a/Documentation/process/index.rst b/Documentation/process/index.rst
> > index aa12f26601949..e1a8a31389f53 100644
> > --- a/Documentation/process/index.rst
> > +++ b/Documentation/process/index.rst
> > @@ -68,6 +68,7 @@ beyond).
> >     stable-kernel-rules
> >     management-style
> >     researcher-guidelines
> > +   generated-content
> >  
> >  Dealing with bugs
> >  -----------------
> 

^ permalink raw reply

* Re: [PATCH] [v2] Documentation: Provide guidelines for tool-generated content
From: Lorenzo Stoakes @ 2025-11-10 11:15 UTC (permalink / raw)
  To: Dave Hansen
  Cc: linux-kernel, Steven Rostedt, Dan Williams, Theodore Ts'o,
	Sasha Levin, Jonathan Corbet, Kees Cook, Greg Kroah-Hartman,
	Miguel Ojeda, Shuah Khan, Christian Brauner, Vlastimil Babka,
	workflows@vger.kernel.org, ksummit@lists.linux.dev, Dan Carpenter,
	Julia Lawall, James Bottomley, Mark Brown, Paul E. McKenney,
	Jiri Kosina
In-Reply-To: <11eaf7fa-27d0-4a57-abf0-5f24c918966c@lucifer.local>

+cc potentially interested parties.

Apologies if I missed anybody, just scanned through quickly.

On Mon, Nov 10, 2025 at 10:48:04AM +0000, Lorenzo Stoakes wrote:
> I think it would have been helpful to ping those engaged in the discussion in
> this area in related threads, e.g. [0] and [1].
>
> [0]: https://lore.kernel.org/ksummit/49f1a974-e1e6-4be5-864e-5e0f905e1a8f@paulmck-laptop/T/#m30873ef3dc9bd2c4c95547e81efff3085474f2d9
> [1]: https://lore.kernel.org/all/7e7f485e-93ad-4bc4-9323-f154ce477c39@lucifer.local/
>
> I'm not sure what the process was that lead to this, but it feels rather as if
> the community were excluded here.
>
> It also seems slightly odd to produce this in advance of the maintainer's
> summit, as I felt there was some agreement that the topic should be discussed
> there?
>
> Obviously there may be very good reasons for this but it'd be good for them to
> be clarified and those who engaged in these discussions to be cc'd also (or at
> least ping on threads linking!)
>
> On Wed, Nov 05, 2025 at 03:15:14PM -0800, Dave Hansen wrote:
> > In the last few years, the capabilities of coding tools have exploded.
> > As those capabilities have expanded, contributors and maintainers have
> > more and more questions about how and when to apply those
> > capabilities.
> >
> > The shiny new AI tools (chatbots, coding assistants and more) are
> > impressive.  Add new Documentation to guide contributors on how to
> > best use kernel development tools, new and old.
>
> As others have pointed out, this is strangely gleeful, can we please drop it?
>
> As mentioned in the msummit thread I have a great concern about how the press
> might report on this kind of change, as I fear that a 'kernel accepts AI
> patches' story might result in a large influx of AI patches from enthusiatic
> people which will have a direct impact on maintainer workload.
>
> I don't think comments like this help in that respect.
>
> In general I feel that a more restrictive/pessmistic document that can later be
> made less pessimistic/restrictive is a better approach than a broad one on this
> basis.
>
> >
> > Note, though, there are fundamentally no new or unique rules in this
> > new document. It clarifies expectations that the kernel community has
>
> Hmm, I'm not sure the conflation of pre-existing tooling which always required
> some degree of understanding vs. a technique which can simply generate entire
> patch sets with commentary included is justified.
>
> While I _do_ like the idea that basic principles that already existed still
> exist for LLMs (that's a powerful notion), I wonder if we do in fact do need
> some new rules here.
>
> I think saying this also pushes back on the concept of maintainer-by-maintainer
> policy as 'it's just like it always was' doesn't suggest that it warrants a
> higher level of scrutiny.
>
> > had for many years. For example, researchers are already asked to
> > disclose the tools they use to find issues in
> > Documentation/process/researcher-guidelines.rst. This new document
> > just reiterates existing best practices for development tooling.
>
> Ironically that document is considerably more strident and firm than this
> one :)
>
> >
> > In short: Please show your work and make sure your contribution is
> > easy to review.
>
> I wonder whether we need to be very explicit in stating - please do not
> generate patches in large volume with no involvement from you and
> _emphasise_ that human involvement is _necessary_.
>
> In discussion with kernel colleagues who use AI extensively, there is a
> very clear pattern than a key part of usefully making use of this tooling
> is for there to be an 'expert in the loop' who reviews what is generated to
> ensure it is correct.
>
> I therefore think we either _should_ have a specific rule for LLM-generated
> content or should (and it really makes sense actually) have a broad
> 'generated content' rule that - you _must_ have a thorough understanding of
> what you are doing such that you can review and filter the generated
> output.
>
> I think stating that we will NOT accept series that are generated without
> understanding would be very beneficial in all respects, rather than leaving
> it somehow implied.
>
> Being soft or vague here is likely to cause maintainer headaches IMO
> (though of course there's only so many who will read a doc etc. being able
> to point at the document in reply as a maintainer is useful too).
>
> >
> > Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: Theodore Ts'o <tytso@mit.edu>
> > Cc: Sasha Levin <sashal@kernel.org>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Kees Cook <kees@kernel.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Miguel Ojeda <ojeda@kernel.org>
> > Cc: Shuah Khan <shuah@kernel.org>
> >
> > --
> >
> > This document was a collaborative effort from all the members of
> > the TAB. I just reformatted it into .rst and wrote the changelog.
> >
> > Changes from v1:
> >  * Rename to generated-content.rst and add to documentation index.
> >    (Jon)
> >  * Rework subject to align with the new filename
> >  * Replace commercial names with generic ones. (Jon)
> >  * Be consistent about punctuation at the end of bullets for whole
> >    sentences. (Miguel)
> >  * Formatting sprucing up and minor typos (Miguel)
> > ---
> >  Documentation/process/generated-content.rst | 94 +++++++++++++++++++++
> >  Documentation/process/index.rst             |  1 +
> >  2 files changed, 95 insertions(+)
> >  create mode 100644 Documentation/process/generated-content.rst
> >
> > diff --git a/Documentation/process/generated-content.rst b/Documentation/process/generated-content.rst
> > new file mode 100644
> > index 0000000000000..5e8ff44190932
> > --- /dev/null
> > +++ b/Documentation/process/generated-content.rst
> > @@ -0,0 +1,94 @@
> > +============================================
> > +Kernel Guidelines for Tool Generated Content
> > +============================================
> > +
> > +Purpose
> > +=======
> > +
> > +Kernel contributors have been using tooling to generate contributions
> > +for a long time. These tools are constantly becoming more capable and
> > +undoubtedly improve developer productivity. At the same time, reviewer
> > +and maintainer bandwidth is a very scarce resource. Understanding
>
> This is absolutely the key issue here imo, maintainer bandwidth. Glad this
> is in the opener.
>
> > +which portions of a contribution come from humans versus tools is
> > +critical to maintain those resources and keep kernel development
> > +healthy.
>
> Agreed entirely.
>
> > +
> > +The goal here is to clarify community expectations around tools. This
> > +lets everyone become more productive while also maintaining high
> > +degrees of trust between submitters and reviewers.
>
> Also very good.
>
> > +
> > +Out of Scope
> > +============
> > +
> > +These guidelines do not apply to tools that make trivial tweaks to
> > +preexisting content. Nor do they pertain to AI tooling that helps with
> > +menial tasks. Some examples:
> > +
> > + - Spelling and grammar fix ups, like rephrasing to imperative voice
> > + - Typing aids like identifier completion, common boilerplate or
> > +   trivial pattern completion
> > + - Purely mechanical transformations like variable renaming
> > + - Reformatting, like running Lindent, ``clang-format`` or
> > +   ``rust-fmt``
> > +
> > +Even if your tool use is out of scope you should still always consider
> > +if it would help reviewing your contribution if the reviewer knows
> > +about the tool that you used.
>
> This is great, I agree very much that we have to be reasonable about these
> uses.
>
> The final sentence is also great.
>
> > +
> > +In Scope
> > +========
> > +
> > +These guidelines apply when a meaningful amount of content in a kernel
> > +contribution was not written by a person in the Signed-off-by chain,
> > +but was instead created by a tool.
>
> Yes, perhaps useful actually using the term 'meaningful amount' rather than
> trying to be absolutely explicit about what this entails.
>
> Also allows for maintainer discretion.
>
> > +
> > +Detection of a problem is also a part of the development process; if a
> > +tool was used to find a problem addressed by a change, that should be
> > +noted in the changelog. This not only gives credit where it is due, it
> > +also helps fellow developers find out about these tools.
> > +
> > +Some examples:
> > + - Any tool-suggested fix such as ``checkpatch.pl --fix``
> > + - Coccinelle scripts
> > + - A chatbot generated a new function in your patch to sort list entries.
> > + - A .c file in the patch was originally generated by a LLM but cleaned
> > +   up by hand.
> > + - The changelog was generated by handing the patch to a generative AI
> > +   tool and asking it to write the changelog.
> > + - The changelog was translated from another language.
> > +
> > +If in doubt, choose transparency and assume these guidelines apply to
> > +your contribution.
>
> Yes agreed.
>
> > +
> > +Guidelines
> > +==========
> > +
> > +First, read the Developer's Certificate of Origin:
> > +Documentation/process/submitting-patches.rst . Its rules are simple
> > +and have been in place for a long time. They have covered many
> > +tool-generated contributions.
> > +
> > +Second, when making a contribution, be transparent about the origin of
> > +content in cover letters and changelogs. You can be more transparent
> > +by adding information like this:
> > +
> > + - What tools were used?
> > + - The input to the tools you used, like the coccinelle source script.
>
> Not sure repeatedly using coccinelle as an example is helpful, as
> coccinelle is far less of an issue than LLM tooling, perhaps for the
> avoidance of doubt, expand this to include references to that?
>
> > + - If code was largely generated from a single or short set of
> > +   prompts, include those prompts in the commit log. For longer
> > +   sessions, include a summary of the prompts and the nature of
> > +   resulting assistance.
>
> Maybe worth saying send it in a cover letter if a series, but perhaps
> pedantic.
>
> > + - Which portions of the content were affected by that tool?
> > +
> > +As with all contributions, individual maintainers have discretion to
> > +choose how they handle the contribution. For example, they might:
> > +
> > + - Treat it just like any other contribution
> > + - Reject it outright
> > + - Review the contribution with extra scrutiny
> > + - Suggest a better prompt instead of suggesting specific code changes
> > + - Ask for some other special steps, like asking the contributor to
> > +   elaborate on how the tool or model was trained
> > + - Ask the submitter to explain in more detail about the contribution
> > +   so that the maintainer can feel comfortable that the submitter fully
> > +   understands how the code works.
>
> OK I wrote something suggesting you add this and you already have :) that's
> great. Let me go delete that request :)
>
> However I'm not sure the 'as with all contributions' is right though - as a
> maintainer in mm I don't actually feel that we can reject outright without
> having to give significant explanation as to why.
>
> And I think that's often the case - people (rightly) dislike blanket NAKs
> and it's a terrible practice, which often (also rightly) gets pushback from
> co-maintainers or others in the community.
>
> So I think perhaps it'd also be useful to very explicitly say that
> maintainers may say no summarily in instances where the review load would
> simply be too much to handle large clearly-AI-generated and
> clearly-unfiltered series.
>
> Another point to raise perhaps is that - even in the cases where the
> submitter is carefully reviewing generated output - that submitters must be
> reasonable in terms of the volume they submit. This is perhaps hand wavey
> but mentioning it would be great not least for the ability for maintainers
> to point at the doc and reference it.
>
> > diff --git a/Documentation/process/index.rst b/Documentation/process/index.rst
> > index aa12f26601949..e1a8a31389f53 100644
> > --- a/Documentation/process/index.rst
> > +++ b/Documentation/process/index.rst
> > @@ -68,6 +68,7 @@ beyond).
> >     stable-kernel-rules
> >     management-style
> >     researcher-guidelines
> > +   generated-content
> >
> >  Dealing with bugs
> >  -----------------
>
> I guess this is a WIP?
>
> > --
> > 2.34.1
> >
> >
>
> Thanks, Lorenzo

^ permalink raw reply

* Re: [PATCH] [v2] Documentation: Provide guidelines for tool-generated content
From: Dave Hansen @ 2025-11-10 16:08 UTC (permalink / raw)
  To: Christian Brauner, Dave Hansen
  Cc: Vlastimil Babka, linux-kernel, workflows@vger.kernel.org,
	ksummit@lists.linux.dev, Steven Rostedt, Dan Williams,
	Theodore Ts'o, Sasha Levin, Jonathan Corbet, Kees Cook,
	Greg Kroah-Hartman, Miguel Ojeda, Shuah Khan
In-Reply-To: <20251110-weiht-etablieren-39e7b63ef76d@brauner>

On 11/10/25 00:58, Christian Brauner wrote:
...
> This reads like a factual statement about "impressiveness" of the tools.
> Just drop that sentence, please. It doesn't add value to the commit
> message at all.\

Sure thing. Dropped.

...>>> These tools are constantly becoming more capable and
>>> +undoubtedly improve developer productivity. At the same time, reviewer
> 
> "undoubtedly improve developer productivity"?
> Am I reading an advert or kernel documentation about the policy how to
> use new tooling?
> 
> Please keep it factual without statements about what perceived value
> this adds. People use it and we have to have a policy for it. There's no
> need to celebrate it.

I can definitely steer this away from perceived value. But the main
point of this section was to do some impedance matching between
maintainers and contributors. You (the contributor) may be more
productive, but the maintainer just got more patches to review.

So we could easily tone this down by changing:

	These tools are constantly becoming more capable and
	undoubtedly improve developer productivity.

to

	These tools can increase the volume of contributions.

But I do think it's important to make the connection between
reviewer/maintainer scarcity and tooling.

^ permalink raw reply

* [PATCH v8 00/27] mm/ksw: Introduce KStackWatch debugging tool
From: Jinchao Wang @ 2025-11-10 16:35 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86

Earlier this year, I debugged a stack corruption panic that revealed the
limitations of existing debugging tools. The bug persisted for 739 days
before being fixed (CVE-2025-22036), and my reproduction scenario
differed from the CVE report—highlighting how unpredictably these bugs
manifest.

The panic call trace:

<4>[89318.486564]  <TASK>
<4>[89318.486570]  dump_stack_lvl+0x48/0x70
<4>[89318.486580]  dump_stack+0x10/0x20
<4>[89318.486586]  panic+0x345/0x3a0
<4>[89318.486596]  ? __blk_flush_plug+0x121/0x130
<4>[89318.486603]  __stack_chk_fail+0x14/0x20
<4>[89318.486612]  __blk_flush_plug+0x121/0x130
...27 other frames omitted
<4>[89318.486824]  ksys_read+0x6b/0xf0
<4>[89318.486829]  __x64_sys_read+0x19/0x30
<4>[89318.486834]  x64_sys_call+0x1ada/0x25c0
<4>[89318.486840]  do_syscall_64+0x7f/0x180
<4>[89318.486847]  ? exc_page_fault+0x94/0x1b0
<4>[89318.486855]  entry_SYSCALL_64_after_hwframe+0x73/0x7b
<4>[89318.486866]  </TASK>

Initially, I enabled KASAN, but the bug did not reproduce. Reviewing the
code in __blk_flush_plug(), I found it difficult to trace all logic
paths due to indirect function calls through function pointers.

I added canary-locating code to obtain the canary address and value,
then inserted extensive debugging code to track canary modifications. I
observed the canary being corrupted between two unrelated assignments,
indicating corruption by another thread—a silent stack corruption bug.

I then added hardware breakpoint (hwbp) code, but still failed to catch
the corruption. After adding PID filters, function parameter filters,
and depth filters, I discovered the corruption occurred in
end_buffer_read_sync() via atomic_dec(&bh->b_count), where bh->b_count
overlapped with __blk_flush_plug()'s canary address. Tracing the bh
lifecycle revealed the root cause in exfat_get_block()—a function not
even present in the panic call trace.

This bug was later assigned CVE-2025-22036
(https://lore.kernel.org/all/2025041658-CVE-2025-22036-6469@gregkh/).
The vulnerability was introduced in commit 11a347fb6cef (March 13, 2023)
and fixed in commit 1bb7ff4204b6 (March 21, 2025)—persisting for 739
days. Notably, my reproduction scenario differed significantly from that
described in the CVE report, highlighting how these bugs manifest
unpredictably across different workloads.

This experience revealed how notoriously difficult stack corruption bugs
are to debug: KASAN cannot reproduce them, call traces are misleading,
and the actual culprit often lies outside the visible call chain. Manual
instrumentation with hardware breakpoints and filters was effective but
extremely time-consuming.

This motivated KStackWatch: automating the debugging workflow I manually
performed, making hardware breakpoint-based stack monitoring readily
available to all kernel developers facing similar issues.

KStackWatch is a lightweight debugging tool to detect kernel stack
corruption in real time. It installs a hardware breakpoint (watchpoint)
at a function's specified offset using kprobe.post_handler and removes
it in fprobe.exit_handler. This covers the full execution window and
reports corruption immediately with time, location, and a call stack.

Beyond automating proven debugging workflows, KStackWatch incorporates
robust engineering to handle complex scenarios like context switches,
recursion, and concurrent execution, making it suitable for broad
debugging use cases.

## Key Features

* Immediate and precise stack corruption detection
* Support for multiple concurrent watchpoints with configurable limits
* Lockless design, usable in any context
* Depth filter for recursive calls
* Low overhead of memory and CPU
* Flexible debugfs configuration with key=val syntax
* Architecture support: x86_64 and arm64
* Auto-canary detection to simplify configuration

## Architecture Support

KStackWatch currently supports x86_64 and arm64. The design is
architecture-agnostic, requiring only:
* Hardware breakpoint modification in atomic context

Arm64 support required only ~20 lines of code(patch 18,19). Future ports
to other architectures (e.g., riscv) should be straightforward for
developers familiar with their hardware breakpoint implementations.

## Performance Impact

Runtime overhead was measured on Intel Core Ultra 5 125H @ 3 GHz running
kernel 6.17, using test4 from patch 24:

     Type                 |   Time (ns)  |  Cycles
     -----------------------------------------------
     entry with watch     |     10892    |   32620
     entry without watch  |       159    |     466
     exit  with watch     |     12541    |   37556
     exit  without watch  |       124    |     369

Comparation with other scenarios:

Mode                        |  CPU Overhead (add)  |  Memory Overhead (add)
----------------------------+----------------------+-------------------------
Compiled but not enabled    |  None                |  ~20 B per task
Enabled, no function hit    |  None                |  ~few hundred B
Func hit, HWBP not toggled  |  ~140 ns per call    |  None
Func hit, HWBP toggled      |  ~11–12 µs per call  |  None

The overhead is minimal, making KStackWatch suitable for production
environments where stack corruption is suspected but kernel rebuilds are not feasible.

## Validation

To validate the approach, this series includes a self-contained test module and
a companion shell script. The module provides several test cases covering
scenarios such as canary overflow, recursive depth tracking, multi-threaded
silent corruption, retaddr overwriten. A detailed workflow example and usage
guide are provided in the documentation (patch 26).

While KStackWatch itself is a new tool and has not yet discovered production
bugs, it automates the exact methodology that I used to manually uncover
CVE-2025-22036. The tool is designed to make this powerful debugging technique
readily available to kernel developers, enabling them to efficiently detect and
diagnose similar stack corruption issues in the future.

---
Patches 1–3 of this series are also used in the wprobe work proposed by
Masami Hiramatsu, so there may be some overlap between our patches.
Patch 3 comes directly from Masami Hiramatsu (thanks).
---

Changelog:
v8:
* Add arm64 support
  * Implement hwbp_reinstall() for arm64.
  * Use single-step mode as default in ksw_watch_handler().
* Add latency measurements for probe handlers.
* Update configuration options
  * Introduce explicit auto_canary parameter.
  * Default watch_len to sizeof(unsigned long) when zero.
  * Replace panic_on_catch with panic_hit ksw_config option.
* Enable KStackWatch in non-debug builds.
* Limit canary search range to the current stack frame when possible.
* Add automatic architecture detection for test parameters.
* Move kstackwatch.h to include/linux/.
* Relocate Kconfig fragments to the kstackwatch/ directory.

v7:
  https://lore.kernel.org/all/20251009105650.168917-1-wangjinchao600@gmail.com/
  * Fix maintainer entry to alphabetical position

v6:
  https://lore.kernel.org/all/20250930024402.1043776-1-wangjinchao600@gmail.com/
  * Replace procfs with debugfs interface
  * Fix typos

v5:
  https://lore.kernel.org/all/20250924115124.194940-1-wangjinchao600@gmail.com/
  * Support key=value input format
  * Support multiple watchpoints
  * Support watching instruction inside loop
  * Support recursion depth tracking with generation
  * Ignore triggers from fprobe trampoline
  * Split watch_on into watch_get and watch_on to fail fast
  * Handle ksw_stack_prepare_watch error
  * Rewrite silent corruption test
  * Add multiple watchpoints test
  * Add an example in documentation

v4:
  https://lore.kernel.org/all/20250912101145.465708-1-wangjinchao600@gmail.com/
  * Solve the lockdep issues with:
    * per-task KStackWatch context to track depth
    * atomic flag to protect watched_addr
  * Use refactored version of arch_reinstall_hw_breakpoint

v3:
  https://lore.kernel.org/all/20250910052335.1151048-1-wangjinchao600@gmail.com/
  * Use modify_wide_hw_breakpoint_local() (from Masami)
  * Add atomic flag to restrict /proc/kstackwatch to a single opener
  * Protect stack probe with an atomic PID flag
  * Handle CPU hotplug for watchpoints
  * Add preempt_disable/enable in ksw_watch_on_local_cpu()
  * Introduce const struct ksw_config *ksw_get_config(void) and use it
  * Switch to global watch_attr, remove struct watch_info
  * Validate local_var_len in parser()
  * Handle case when canary is not found
  * Use dump_stack() instead of show_regs() to allow module build
  * Reduce logging and comments
  * Format logs with KBUILD_MODNAME
  * Remove unused headers
  * Add new document

v2:
  https://lore.kernel.org/all/20250904002126.1514566-1-wangjinchao600@gmail.com/
  * Make hardware breakpoint and stack operations
    architecture-independent.

v1:
  https://lore.kernel.org/all/20250828073311.1116593-1-wangjinchao600@gmail.com/
  * Replaced kretprobe with fprobe for function exit hooking, as
    suggested by Masami Hiramatsu
  * Introduced per-task depth logic to track recursion across scheduling
  * Removed the use of workqueue for a more efficient corruption check
  * Reordered patches for better logical flow
  * Simplified and improved commit messages throughout the series
  * Removed initial archcheck which should be improved later
  * Replaced the multiple-thread test with silent corruption test
  * Split self-tests into a separate patch to improve clarity.
  * Added a new entry for KStackWatch to the MAINTAINERS file.
---

Jinchao Wang (26):
  x86/hw_breakpoint: Unify breakpoint install/uninstall
  x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint
  mm/ksw: add build system support
  mm/ksw: add ksw_config struct and parser
  mm/ksw: add singleton debugfs interface
  mm/ksw: add HWBP pre-allocation
  mm/ksw: Add atomic watchpoint management api
  mm/ksw: ignore false positives from exit trampolines
  mm/ksw: support CPU hotplug
  sched/ksw: add per-task context
  mm/ksw: add entry kprobe and exit fprobe management
  mm/ksw: add per-task ctx tracking
  mm/ksw: resolve stack watch addr and len
  mm/ksw: limit canary search to current stack frame
  mm/ksw: manage probe and HWBP lifecycle via procfs
  mm/ksw: add KSTACKWATCH_PROFILING to measure probe cost
  arm64/hw_breakpoint: Add arch_reinstall_hw_breakpoint
  arm64/hwbp/ksw: integrate KStackWatch handler support
  mm/ksw: add self-debug helpers
  mm/ksw: add test module
  mm/ksw: add stack overflow test
  mm/ksw: add recursive depth test
  mm/ksw: add multi-thread corruption test cases
  tools/ksw: add arch-specific test script
  docs: add KStackWatch document
  MAINTAINERS: add entry for KStackWatch

Masami Hiramatsu (Google) (1):
  HWBP: Add modify_wide_hw_breakpoint_local() API

 Documentation/dev-tools/index.rst       |   1 +
 Documentation/dev-tools/kstackwatch.rst | 377 +++++++++++++++++++++
 MAINTAINERS                             |   9 +
 arch/Kconfig                            |  10 +
 arch/arm64/Kconfig                      |   1 +
 arch/arm64/include/asm/hw_breakpoint.h  |   1 +
 arch/arm64/kernel/hw_breakpoint.c       |  12 +
 arch/x86/Kconfig                        |   1 +
 arch/x86/include/asm/hw_breakpoint.h    |   8 +
 arch/x86/kernel/hw_breakpoint.c         | 148 +++++----
 include/linux/hw_breakpoint.h           |   6 +
 include/linux/kstackwatch.h             |  68 ++++
 include/linux/kstackwatch_types.h       |  14 +
 include/linux/sched.h                   |   5 +
 kernel/events/hw_breakpoint.c           |  37 +++
 mm/Kconfig                              |   1 +
 mm/Makefile                             |   1 +
 mm/kstackwatch/Kconfig                  |  34 ++
 mm/kstackwatch/Makefile                 |   8 +
 mm/kstackwatch/kernel.c                 | 295 +++++++++++++++++
 mm/kstackwatch/stack.c                  | 416 ++++++++++++++++++++++++
 mm/kstackwatch/test.c                   | 345 ++++++++++++++++++++
 mm/kstackwatch/watch.c                  | 309 ++++++++++++++++++
 tools/kstackwatch/kstackwatch_test.sh   |  85 +++++
 24 files changed, 2130 insertions(+), 62 deletions(-)
 create mode 100644 Documentation/dev-tools/kstackwatch.rst
 create mode 100644 include/linux/kstackwatch.h
 create mode 100644 include/linux/kstackwatch_types.h
 create mode 100644 mm/kstackwatch/Kconfig
 create mode 100644 mm/kstackwatch/Makefile
 create mode 100644 mm/kstackwatch/kernel.c
 create mode 100644 mm/kstackwatch/stack.c
 create mode 100644 mm/kstackwatch/test.c
 create mode 100644 mm/kstackwatch/watch.c
 create mode 100755 tools/kstackwatch/kstackwatch_test.sh

-* 
2.43.0


^ permalink raw reply

* [PATCH v8 01/27] x86/hw_breakpoint: Unify breakpoint install/uninstall
From: Jinchao Wang @ 2025-11-10 16:35 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

Consolidate breakpoint management to reduce code duplication.
The diffstat was misleading, so the stripped code size is compared instead.
After refactoring, it is reduced from 11976 bytes to 11448 bytes on my
x86_64 system built with clang.

This also makes it easier to introduce arch_reinstall_hw_breakpoint().

In addition, including linux/types.h to fix a missing build dependency.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 arch/x86/include/asm/hw_breakpoint.h |   6 ++
 arch/x86/kernel/hw_breakpoint.c      | 141 +++++++++++++++------------
 2 files changed, 84 insertions(+), 63 deletions(-)

diff --git a/arch/x86/include/asm/hw_breakpoint.h b/arch/x86/include/asm/hw_breakpoint.h
index 0bc931cd0698..aa6adac6c3a2 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -5,6 +5,7 @@
 #include <uapi/asm/hw_breakpoint.h>
 
 #define	__ARCH_HW_BREAKPOINT_H
+#include <linux/types.h>
 
 /*
  * The name should probably be something dealt in
@@ -18,6 +19,11 @@ struct arch_hw_breakpoint {
 	u8		type;
 };
 
+enum bp_slot_action {
+	BP_SLOT_ACTION_INSTALL,
+	BP_SLOT_ACTION_UNINSTALL,
+};
+
 #include <linux/kdebug.h>
 #include <linux/percpu.h>
 #include <linux/list.h>
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index b01644c949b2..3658ace4bd8d 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -48,7 +48,6 @@ static DEFINE_PER_CPU(unsigned long, cpu_debugreg[HBP_NUM]);
  */
 static DEFINE_PER_CPU(struct perf_event *, bp_per_reg[HBP_NUM]);
 
-
 static inline unsigned long
 __encode_dr7(int drnum, unsigned int len, unsigned int type)
 {
@@ -85,96 +84,112 @@ int decode_dr7(unsigned long dr7, int bpnum, unsigned *len, unsigned *type)
 }
 
 /*
- * Install a perf counter breakpoint.
- *
- * We seek a free debug address register and use it for this
- * breakpoint. Eventually we enable it in the debug control register.
- *
- * Atomic: we hold the counter->ctx->lock and we only handle variables
- * and registers local to this cpu.
+ * We seek a slot and change it or keep it based on the action.
+ * Returns slot number on success, negative error on failure.
+ * Must be called with IRQs disabled.
  */
-int arch_install_hw_breakpoint(struct perf_event *bp)
+static int manage_bp_slot(struct perf_event *bp, enum bp_slot_action action)
 {
-	struct arch_hw_breakpoint *info = counter_arch_bp(bp);
-	unsigned long *dr7;
-	int i;
-
-	lockdep_assert_irqs_disabled();
+	struct perf_event *old_bp;
+	struct perf_event *new_bp;
+	int slot;
+
+	switch (action) {
+	case BP_SLOT_ACTION_INSTALL:
+		old_bp = NULL;
+		new_bp = bp;
+		break;
+	case BP_SLOT_ACTION_UNINSTALL:
+		old_bp = bp;
+		new_bp = NULL;
+		break;
+	default:
+		return -EINVAL;
+	}
 
-	for (i = 0; i < HBP_NUM; i++) {
-		struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
+	for (slot = 0; slot < HBP_NUM; slot++) {
+		struct perf_event **curr = this_cpu_ptr(&bp_per_reg[slot]);
 
-		if (!*slot) {
-			*slot = bp;
-			break;
+		if (*curr == old_bp) {
+			*curr = new_bp;
+			return slot;
 		}
 	}
 
-	if (WARN_ONCE(i == HBP_NUM, "Can't find any breakpoint slot"))
-		return -EBUSY;
+	if (old_bp) {
+		WARN_ONCE(1, "Can't find matching breakpoint slot");
+		return -EINVAL;
+	}
+
+	WARN_ONCE(1, "No free breakpoint slots");
+	return -EBUSY;
+}
+
+static void setup_hwbp(struct arch_hw_breakpoint *info, int slot, bool enable)
+{
+	unsigned long dr7;
 
-	set_debugreg(info->address, i);
-	__this_cpu_write(cpu_debugreg[i], info->address);
+	set_debugreg(info->address, slot);
+	__this_cpu_write(cpu_debugreg[slot], info->address);
 
-	dr7 = this_cpu_ptr(&cpu_dr7);
-	*dr7 |= encode_dr7(i, info->len, info->type);
+	dr7 = this_cpu_read(cpu_dr7);
+	if (enable)
+		dr7 |= encode_dr7(slot, info->len, info->type);
+	else
+		dr7 &= ~__encode_dr7(slot, info->len, info->type);
 
 	/*
-	 * Ensure we first write cpu_dr7 before we set the DR7 register.
-	 * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+	 * Enabling:
+	 *   Ensure we first write cpu_dr7 before we set the DR7 register.
+	 *   This ensures an NMI never see cpu_dr7 0 when DR7 is not.
 	 */
+	if (enable)
+		this_cpu_write(cpu_dr7, dr7);
+
 	barrier();
 
-	set_debugreg(*dr7, 7);
+	set_debugreg(dr7, 7);
+
 	if (info->mask)
-		amd_set_dr_addr_mask(info->mask, i);
+		amd_set_dr_addr_mask(enable ? info->mask : 0, slot);
 
-	return 0;
+	/*
+	 * Disabling:
+	 *   Ensure the write to cpu_dr7 is after we've set the DR7 register.
+	 *   This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+	 */
+	if (!enable)
+		this_cpu_write(cpu_dr7, dr7);
 }
 
 /*
- * Uninstall the breakpoint contained in the given counter.
- *
- * First we search the debug address register it uses and then we disable
- * it.
- *
- * Atomic: we hold the counter->ctx->lock and we only handle variables
- * and registers local to this cpu.
+ * find suitable breakpoint slot and set it up based on the action
  */
-void arch_uninstall_hw_breakpoint(struct perf_event *bp)
+static int arch_manage_bp(struct perf_event *bp, enum bp_slot_action action)
 {
-	struct arch_hw_breakpoint *info = counter_arch_bp(bp);
-	unsigned long dr7;
-	int i;
+	struct arch_hw_breakpoint *info;
+	int slot;
 
 	lockdep_assert_irqs_disabled();
 
-	for (i = 0; i < HBP_NUM; i++) {
-		struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
-
-		if (*slot == bp) {
-			*slot = NULL;
-			break;
-		}
-	}
-
-	if (WARN_ONCE(i == HBP_NUM, "Can't find any breakpoint slot"))
-		return;
+	slot = manage_bp_slot(bp, action);
+	if (slot < 0)
+		return slot;
 
-	dr7 = this_cpu_read(cpu_dr7);
-	dr7 &= ~__encode_dr7(i, info->len, info->type);
+	info = counter_arch_bp(bp);
+	setup_hwbp(info, slot, action != BP_SLOT_ACTION_UNINSTALL);
 
-	set_debugreg(dr7, 7);
-	if (info->mask)
-		amd_set_dr_addr_mask(0, i);
+	return 0;
+}
 
-	/*
-	 * Ensure the write to cpu_dr7 is after we've set the DR7 register.
-	 * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
-	 */
-	barrier();
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+	return arch_manage_bp(bp, BP_SLOT_ACTION_INSTALL);
+}
 
-	this_cpu_write(cpu_dr7, dr7);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp)
+{
+	arch_manage_bp(bp, BP_SLOT_ACTION_UNINSTALL);
 }
 
 static int arch_bp_generic_len(int x86_len)
-- 
2.43.0


^ permalink raw reply related

* [PATCH v8 02/27] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint
From: Jinchao Wang @ 2025-11-10 16:35 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

The new arch_reinstall_hw_breakpoint() function can be used in an
atomic context, unlike the more expensive free and re-allocation path.
This allows callers to efficiently re-establish an existing breakpoint.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 arch/x86/include/asm/hw_breakpoint.h | 2 ++
 arch/x86/kernel/hw_breakpoint.c      | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/arch/x86/include/asm/hw_breakpoint.h b/arch/x86/include/asm/hw_breakpoint.h
index aa6adac6c3a2..c22cc4e87fc5 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -21,6 +21,7 @@ struct arch_hw_breakpoint {
 
 enum bp_slot_action {
 	BP_SLOT_ACTION_INSTALL,
+	BP_SLOT_ACTION_REINSTALL,
 	BP_SLOT_ACTION_UNINSTALL,
 };
 
@@ -65,6 +66,7 @@ extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
 
 
 int arch_install_hw_breakpoint(struct perf_event *bp);
+int arch_reinstall_hw_breakpoint(struct perf_event *bp);
 void arch_uninstall_hw_breakpoint(struct perf_event *bp);
 void hw_breakpoint_pmu_read(struct perf_event *bp);
 void hw_breakpoint_pmu_unthrottle(struct perf_event *bp);
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 3658ace4bd8d..29c9369264d4 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -99,6 +99,10 @@ static int manage_bp_slot(struct perf_event *bp, enum bp_slot_action action)
 		old_bp = NULL;
 		new_bp = bp;
 		break;
+	case BP_SLOT_ACTION_REINSTALL:
+		old_bp = bp;
+		new_bp = bp;
+		break;
 	case BP_SLOT_ACTION_UNINSTALL:
 		old_bp = bp;
 		new_bp = NULL;
@@ -187,6 +191,11 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
 	return arch_manage_bp(bp, BP_SLOT_ACTION_INSTALL);
 }
 
+int arch_reinstall_hw_breakpoint(struct perf_event *bp)
+{
+	return arch_manage_bp(bp, BP_SLOT_ACTION_REINSTALL);
+}
+
 void arch_uninstall_hw_breakpoint(struct perf_event *bp)
 {
 	arch_manage_bp(bp, BP_SLOT_ACTION_UNINSTALL);
-- 
2.43.0


^ permalink raw reply related

* [PATCH v8 03/27] HWBP: Add modify_wide_hw_breakpoint_local() API
From: Jinchao Wang @ 2025-11-10 16:35 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>

Add modify_wide_hw_breakpoint_local() arch-wide interface which allows
hwbp users to update watch address on-line. This is available if the
arch supports CONFIG_HAVE_REINSTALL_HW_BREAKPOINT.
Note that this allows to change the type only for compatible types,
because it does not release and reserve the hwbp slot based on type.
For instance, you can not change HW_BREAKPOINT_W to HW_BREAKPOINT_X.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 arch/Kconfig                  | 10 ++++++++++
 arch/x86/Kconfig              |  1 +
 include/linux/hw_breakpoint.h |  6 ++++++
 kernel/events/hw_breakpoint.c | 37 +++++++++++++++++++++++++++++++++++
 4 files changed, 54 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 61130b88964b..c45fe5366125 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -456,6 +456,16 @@ config HAVE_MIXED_BREAKPOINTS_REGS
 	  Select this option if your arch implements breakpoints under the
 	  latter fashion.
 
+config HAVE_REINSTALL_HW_BREAKPOINT
+	bool
+	depends on HAVE_HW_BREAKPOINT
+	help
+	  Depending on the arch implementation of hardware breakpoints,
+	  some of them are able to update the breakpoint configuration
+	  without release and reserve the hardware breakpoint register.
+	  What configuration is able to update depends on hardware and
+	  software implementation.
+
 config HAVE_USER_RETURN_NOTIFIER
 	bool
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index fa3b616af03a..4d2ef8a45681 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -245,6 +245,7 @@ config X86
 	select HAVE_FUNCTION_TRACER
 	select HAVE_GCC_PLUGINS
 	select HAVE_HW_BREAKPOINT
+	select HAVE_REINSTALL_HW_BREAKPOINT
 	select HAVE_IOREMAP_PROT
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK	if X86_64
 	select HAVE_IRQ_TIME_ACCOUNTING
diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index db199d653dd1..ea373f2587f8 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -81,6 +81,9 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
 			    perf_overflow_handler_t triggered,
 			    void *context);
 
+extern int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+					   struct perf_event_attr *attr);
+
 extern int register_perf_hw_breakpoint(struct perf_event *bp);
 extern void unregister_hw_breakpoint(struct perf_event *bp);
 extern void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events);
@@ -124,6 +127,9 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
 			    perf_overflow_handler_t triggered,
 			    void *context)		{ return NULL; }
 static inline int
+modify_wide_hw_breakpoint_local(struct perf_event *bp,
+				struct perf_event_attr *attr) { return -ENOSYS; }
+static inline int
 register_perf_hw_breakpoint(struct perf_event *bp)	{ return -ENOSYS; }
 static inline void unregister_hw_breakpoint(struct perf_event *bp)	{ }
 static inline void
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 8ec2cb688903..5ee1522a99c9 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -887,6 +887,43 @@ void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events)
 }
 EXPORT_SYMBOL_GPL(unregister_wide_hw_breakpoint);
 
+/**
+ * modify_wide_hw_breakpoint_local - update breakpoint config for local CPU
+ * @bp: the hwbp perf event for this CPU
+ * @attr: the new attribute for @bp
+ *
+ * This does not release and reserve the slot of a HWBP; it just reuses the
+ * current slot on local CPU. So the users must update the other CPUs by
+ * themselves.
+ * Also, since this does not release/reserve the slot, this can not change the
+ * type to incompatible type of the HWBP.
+ * Return err if attr is invalid or the CPU fails to update debug register
+ * for new @attr.
+ */
+#ifdef CONFIG_HAVE_REINSTALL_HW_BREAKPOINT
+int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+				    struct perf_event_attr *attr)
+{
+	int ret;
+
+	if (find_slot_idx(bp->attr.bp_type) != find_slot_idx(attr->bp_type))
+		return -EINVAL;
+
+	ret = hw_breakpoint_arch_parse(bp, attr, counter_arch_bp(bp));
+	if (ret)
+		return ret;
+
+	return arch_reinstall_hw_breakpoint(bp);
+}
+#else
+int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+				    struct perf_event_attr *attr)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+EXPORT_SYMBOL_GPL(modify_wide_hw_breakpoint_local);
+
 /**
  * hw_breakpoint_is_used - check if breakpoints are currently used
  *
-- 
2.43.0


^ permalink raw reply related

* [PATCH v8 04/27] mm/ksw: add build system support
From: Jinchao Wang @ 2025-11-10 16:35 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

Add Kconfig and Makefile infrastructure.

The implementation is located under `mm/kstackwatch/`.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 include/linux/kstackwatch.h |  5 +++++
 mm/Kconfig                  |  1 +
 mm/Makefile                 |  1 +
 mm/kstackwatch/Kconfig      | 14 ++++++++++++++
 mm/kstackwatch/Makefile     |  2 ++
 mm/kstackwatch/kernel.c     | 23 +++++++++++++++++++++++
 mm/kstackwatch/stack.c      |  1 +
 mm/kstackwatch/watch.c      |  1 +
 8 files changed, 48 insertions(+)
 create mode 100644 include/linux/kstackwatch.h
 create mode 100644 mm/kstackwatch/Kconfig
 create mode 100644 mm/kstackwatch/Makefile
 create mode 100644 mm/kstackwatch/kernel.c
 create mode 100644 mm/kstackwatch/stack.c
 create mode 100644 mm/kstackwatch/watch.c

diff --git a/include/linux/kstackwatch.h b/include/linux/kstackwatch.h
new file mode 100644
index 000000000000..0273ef478a26
--- /dev/null
+++ b/include/linux/kstackwatch.h
@@ -0,0 +1,5 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _KSTACKWATCH_H
+#define _KSTACKWATCH_H
+
+#endif /* _KSTACKWATCH_H */
diff --git a/mm/Kconfig b/mm/Kconfig
index 0e26f4fc8717..61d4e6edadf2 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -1373,5 +1373,6 @@ config FIND_NORMAL_PAGE
 	def_bool n
 
 source "mm/damon/Kconfig"
+source "mm/kstackwatch/Kconfig"
 
 endmenu
diff --git a/mm/Makefile b/mm/Makefile
index 21abb3353550..efc101816f00 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_PAGE_POISONING) += page_poison.o
 obj-$(CONFIG_KASAN)	+= kasan/
 obj-$(CONFIG_KFENCE) += kfence/
 obj-$(CONFIG_KMSAN)	+= kmsan/
+obj-$(CONFIG_KSTACKWATCH)	+= kstackwatch/
 obj-$(CONFIG_FAILSLAB) += failslab.o
 obj-$(CONFIG_FAIL_PAGE_ALLOC) += fail_page_alloc.o
 obj-$(CONFIG_MEMTEST)		+= memtest.o
diff --git a/mm/kstackwatch/Kconfig b/mm/kstackwatch/Kconfig
new file mode 100644
index 000000000000..496caf264f35
--- /dev/null
+++ b/mm/kstackwatch/Kconfig
@@ -0,0 +1,14 @@
+config KSTACKWATCH
+	bool "Kernel Stack Watch"
+	depends on HAVE_HW_BREAKPOINT && KPROBES && FPROBE && STACKTRACE
+	help
+	  A lightweight real-time debugging tool to detect stack corruption
+	  and abnormal stack usage patterns in the kernel. It monitors stack
+	  boundaries and detects overwrites in real time using hardware
+	  breakpoints and probe-based instrumentation.
+
+	  This feature is intended for kernel developers or advanced users
+	  diagnosing rare stack overflow or memory corruption bugs. It may
+	  introduce minor overhead during runtime monitoring.
+
+	  If unsure, say N.
diff --git a/mm/kstackwatch/Makefile b/mm/kstackwatch/Makefile
new file mode 100644
index 000000000000..c99c621eac02
--- /dev/null
+++ b/mm/kstackwatch/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_KSTACKWATCH)	+= kstackwatch.o
+kstackwatch-y := kernel.o stack.o watch.o
diff --git a/mm/kstackwatch/kernel.c b/mm/kstackwatch/kernel.c
new file mode 100644
index 000000000000..78f1d019225f
--- /dev/null
+++ b/mm/kstackwatch/kernel.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+
+static int __init kstackwatch_init(void)
+{
+	pr_info("module loaded\n");
+	return 0;
+}
+
+static void __exit kstackwatch_exit(void)
+{
+	pr_info("module unloaded\n");
+}
+
+module_init(kstackwatch_init);
+module_exit(kstackwatch_exit);
+
+MODULE_AUTHOR("Jinchao Wang");
+MODULE_DESCRIPTION("Kernel Stack Watch");
+MODULE_LICENSE("GPL");
+
diff --git a/mm/kstackwatch/stack.c b/mm/kstackwatch/stack.c
new file mode 100644
index 000000000000..cec594032515
--- /dev/null
+++ b/mm/kstackwatch/stack.c
@@ -0,0 +1 @@
+// SPDX-License-Identifier: GPL-2.0
diff --git a/mm/kstackwatch/watch.c b/mm/kstackwatch/watch.c
new file mode 100644
index 000000000000..cec594032515
--- /dev/null
+++ b/mm/kstackwatch/watch.c
@@ -0,0 +1 @@
+// SPDX-License-Identifier: GPL-2.0
-- 
2.43.0


^ permalink raw reply related

* [PATCH v8 05/27] mm/ksw: add ksw_config struct and parser
From: Jinchao Wang @ 2025-11-10 16:36 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

Add struct ksw_config and ksw_parse_config() to parse user string.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 include/linux/kstackwatch.h |  33 +++++++++++
 mm/kstackwatch/kernel.c     | 114 ++++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+)

diff --git a/include/linux/kstackwatch.h b/include/linux/kstackwatch.h
index 0273ef478a26..dd00c4c8922e 100644
--- a/include/linux/kstackwatch.h
+++ b/include/linux/kstackwatch.h
@@ -2,4 +2,37 @@
 #ifndef _KSTACKWATCH_H
 #define _KSTACKWATCH_H
 
+#include <linux/types.h>
+
+#define MAX_CONFIG_STR_LEN 128
+
+struct ksw_config {
+	char *func_name;
+	u16 depth;
+
+	/*
+	 * watched variable info:
+	 * - func_offset : instruction offset in the function, typically the
+	 *                 assignment of the watched variable, where ksw
+	 *                 registers a kprobe post-handler.
+	 * - sp_offset   : offset from stack pointer at func_offset. Usually 0.
+	 * - watch_len   : size of the watched variable (1, 2, 4, or 8 bytes).
+	 */
+	u16 func_offset;
+	u16 sp_offset;
+	u16 watch_len;
+
+	/* max number of hwbps that can be used */
+	u16 max_watch;
+
+	/* search canary as watch target automatically */
+	u16 auto_canary;
+
+	/* panic on watchpoint hit */
+	u16 panic_hit;
+
+	/* save to show */
+	char *user_input;
+};
+
 #endif /* _KSTACKWATCH_H */
diff --git a/mm/kstackwatch/kernel.c b/mm/kstackwatch/kernel.c
index 78f1d019225f..50104e78cf3d 100644
--- a/mm/kstackwatch/kernel.c
+++ b/mm/kstackwatch/kernel.c
@@ -1,16 +1,130 @@
 // SPDX-License-Identifier: GPL-2.0
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/kstackwatch.h>
+#include <linux/kstrtox.h>
+#include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/string.h>
+
+static struct ksw_config *ksw_config;
+
+struct param_map {
+	const char *name;       /* long name */
+	const char *short_name; /* short name (2 letters) */
+	size_t offset;          /* offsetof(struct ksw_config, field) */
+	bool is_string;         /* true for string */
+};
+
+/* macro generates both long and short name automatically */
+#define PMAP(field, short, is_str) \
+	{ #field, #short, offsetof(struct ksw_config, field), is_str }
+
+static const struct param_map ksw_params[] = {
+	PMAP(func_name,   fn, true),
+	PMAP(func_offset, fo, false),
+	PMAP(depth,       dp, false),
+	PMAP(max_watch,   mw, false),
+	PMAP(sp_offset,   so, false),
+	PMAP(watch_len,   wl, false),
+	PMAP(auto_canary, ac, false),
+	PMAP(panic_hit,   ph, false),
+};
+
+static int ksw_parse_param(struct ksw_config *config, const char *key,
+			   const char *val)
+{
+	const struct param_map *pm = NULL;
+	int ret;
+
+	for (int i = 0; i < ARRAY_SIZE(ksw_params); i++) {
+		if (strcmp(key, ksw_params[i].name) == 0 ||
+		    strcmp(key, ksw_params[i].short_name) == 0) {
+			pm = &ksw_params[i];
+			break;
+		}
+	}
+
+	if (!pm)
+		return -EINVAL;
+
+	if (pm->is_string) {
+		char **dst = (char **)((char *)config + pm->offset);
+		*dst = kstrdup(val, GFP_KERNEL);
+		if (!*dst)
+			return -ENOMEM;
+	} else {
+		ret = kstrtou16(val, 0, (u16 *)((char *)config + pm->offset));
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/*
+ * Configuration string format:
+ *    param_name=<value> [param_name=<value> ...]
+ *
+ * Required parameters:
+ * - func_name  |fn (str) : target function name
+ * - func_offset|fo (u16) : instruction pointer offset
+ *
+ * Optional parameters:
+ * - depth      |dp (u16) : recursion depth
+ * - max_watch  |mw (u16) : maximum number of watchpoints
+ * - sp_offset  |so (u16) : offset from stack pointer at func_offset
+ * - watch_len  |wl (u16) : watch length (1,2,4,8)
+ */
+static int __maybe_unused ksw_parse_config(char *buf, struct ksw_config *config)
+{
+	char *part, *key, *val;
+	int ret;
+
+	kfree(config->func_name);
+	kfree(config->user_input);
+	memset(ksw_config, 0, sizeof(*ksw_config));
+
+	buf = strim(buf);
+	config->user_input = kstrdup(buf, GFP_KERNEL);
+	if (!config->user_input)
+		return -ENOMEM;
+
+	while ((part = strsep(&buf, " \t\n")) != NULL) {
+		if (*part == '\0')
+			continue;
+
+		key = strsep(&part, "=");
+		val = part;
+		if (!key || !val)
+			continue;
+		ret = ksw_parse_param(config, key, val);
+		if (ret)
+			pr_warn("unsupported param %s=%s", key, val);
+	}
+
+	if (!config->func_name) {
+		pr_err("Missing required parameters: function or func_offset\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
 
 static int __init kstackwatch_init(void)
 {
+	ksw_config = kzalloc(sizeof(*ksw_config), GFP_KERNEL);
+	if (!ksw_config)
+		return -ENOMEM;
+
 	pr_info("module loaded\n");
 	return 0;
 }
 
 static void __exit kstackwatch_exit(void)
 {
+	kfree(ksw_config);
+
 	pr_info("module unloaded\n");
 }
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH v8 06/27] mm/ksw: add singleton debugfs interface
From: Jinchao Wang @ 2025-11-10 16:36 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

Provide the debugfs config file to read or update the configuration.
Only a single process can open this file at a time, enforced using atomic
config_file_busy, to prevent concurrent access.

ksw_get_config() exposes the configuration pointer as const.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 include/linux/kstackwatch.h |   3 ++
 mm/kstackwatch/kernel.c     | 103 ++++++++++++++++++++++++++++++++++--
 2 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/include/linux/kstackwatch.h b/include/linux/kstackwatch.h
index dd00c4c8922e..ada5ac64190c 100644
--- a/include/linux/kstackwatch.h
+++ b/include/linux/kstackwatch.h
@@ -35,4 +35,7 @@ struct ksw_config {
 	char *user_input;
 };
 
+// singleton, only modified in kernel.c
+const struct ksw_config *ksw_get_config(void);
+
 #endif /* _KSTACKWATCH_H */
diff --git a/mm/kstackwatch/kernel.c b/mm/kstackwatch/kernel.c
index 50104e78cf3d..87fef139f494 100644
--- a/mm/kstackwatch/kernel.c
+++ b/mm/kstackwatch/kernel.c
@@ -1,13 +1,18 @@
 // SPDX-License-Identifier: GPL-2.0
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/debugfs.h>
 #include <linux/kstackwatch.h>
 #include <linux/kstrtox.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/string.h>
+#include <linux/uaccess.h>
 
+static atomic_t dbgfs_config_busy = ATOMIC_INIT(0);
 static struct ksw_config *ksw_config;
+static struct dentry *dbgfs_config;
+static struct dentry *dbgfs_dir;
 
 struct param_map {
 	const char *name;       /* long name */
@@ -76,7 +81,7 @@ static int ksw_parse_param(struct ksw_config *config, const char *key,
  * - sp_offset  |so (u16) : offset from stack pointer at func_offset
  * - watch_len  |wl (u16) : watch length (1,2,4,8)
  */
-static int __maybe_unused ksw_parse_config(char *buf, struct ksw_config *config)
+static int ksw_parse_config(char *buf, struct ksw_config *config)
 {
 	char *part, *key, *val;
 	int ret;
@@ -111,18 +116,110 @@ static int __maybe_unused ksw_parse_config(char *buf, struct ksw_config *config)
 	return 0;
 }
 
+static ssize_t ksw_dbgfs_read(struct file *file, char __user *buf, size_t count,
+			      loff_t *ppos)
+{
+	return simple_read_from_buffer(buf, count, ppos, ksw_config->user_input,
+		ksw_config->user_input ? strlen(ksw_config->user_input) : 0);
+}
+
+static ssize_t ksw_dbgfs_write(struct file *file, const char __user *buffer,
+			       size_t count, loff_t *ppos)
+{
+	char input[MAX_CONFIG_STR_LEN];
+	int ret;
+
+	if (count == 0 || count >= sizeof(input))
+		return -EINVAL;
+
+	if (copy_from_user(input, buffer, count))
+		return -EFAULT;
+
+	input[count] = '\0';
+	strim(input);
+
+	if (!strlen(input)) {
+		pr_info("config cleared\n");
+		return count;
+	}
+
+	ret = ksw_parse_config(input, ksw_config);
+	if (ret) {
+		pr_err("Failed to parse config %d\n", ret);
+		return ret;
+	}
+
+	return count;
+}
+
+static int ksw_dbgfs_open(struct inode *inode, struct file *file)
+{
+	if (atomic_cmpxchg(&dbgfs_config_busy, 0, 1))
+		return -EBUSY;
+	return 0;
+}
+
+static int ksw_dbgfs_release(struct inode *inode, struct file *file)
+{
+	atomic_set(&dbgfs_config_busy, 0);
+	return 0;
+}
+
+static const struct file_operations kstackwatch_fops = {
+	.owner = THIS_MODULE,
+	.open = ksw_dbgfs_open,
+	.read = ksw_dbgfs_read,
+	.write = ksw_dbgfs_write,
+	.release = ksw_dbgfs_release,
+	.llseek = default_llseek,
+};
+
+const struct ksw_config *ksw_get_config(void)
+{
+	return ksw_config;
+}
+
 static int __init kstackwatch_init(void)
 {
+	int ret = 0;
+
 	ksw_config = kzalloc(sizeof(*ksw_config), GFP_KERNEL);
-	if (!ksw_config)
-		return -ENOMEM;
+	if (!ksw_config) {
+		ret = -ENOMEM;
+		goto err_alloc;
+	}
+
+	dbgfs_dir = debugfs_create_dir("kstackwatch", NULL);
+	if (!dbgfs_dir) {
+		ret = -ENOMEM;
+		goto err_dir;
+	}
+
+	dbgfs_config = debugfs_create_file("config", 0600, dbgfs_dir, NULL,
+				       &kstackwatch_fops);
+	if (!dbgfs_config) {
+		ret = -ENOMEM;
+		goto err_file;
+	}
 
 	pr_info("module loaded\n");
 	return 0;
+
+err_file:
+	debugfs_remove_recursive(dbgfs_dir);
+	dbgfs_dir = NULL;
+err_dir:
+	kfree(ksw_config);
+	ksw_config = NULL;
+err_alloc:
+	return ret;
 }
 
 static void __exit kstackwatch_exit(void)
 {
+	debugfs_remove_recursive(dbgfs_dir);
+	kfree(ksw_config->func_name);
+	kfree(ksw_config->user_input);
 	kfree(ksw_config);
 
 	pr_info("module unloaded\n");
-- 
2.43.0


^ permalink raw reply related

* [PATCH v8 07/27] mm/ksw: add HWBP pre-allocation
From: Jinchao Wang @ 2025-11-10 16:36 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

Pre-allocate per-CPU hardware breakpoints at init with a place holder
address, which will be retargeted dynamically in kprobe handler.
This avoids allocation in atomic context.

At most max_watch breakpoints are allocated (0 means no limit).

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 include/linux/kstackwatch.h | 13 ++++++
 mm/kstackwatch/watch.c      | 93 +++++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+)

diff --git a/include/linux/kstackwatch.h b/include/linux/kstackwatch.h
index ada5ac64190c..eb9f2b4f2109 100644
--- a/include/linux/kstackwatch.h
+++ b/include/linux/kstackwatch.h
@@ -2,6 +2,9 @@
 #ifndef _KSTACKWATCH_H
 #define _KSTACKWATCH_H
 
+#include <linux/llist.h>
+#include <linux/percpu.h>
+#include <linux/perf_event.h>
 #include <linux/types.h>
 
 #define MAX_CONFIG_STR_LEN 128
@@ -38,4 +41,14 @@ struct ksw_config {
 // singleton, only modified in kernel.c
 const struct ksw_config *ksw_get_config(void);
 
+/* watch management */
+struct ksw_watchpoint {
+	struct perf_event *__percpu *event;
+	struct perf_event_attr attr;
+	struct llist_node node; // for atomic watch_on and off
+	struct list_head list; // for cpu online and offline
+};
+int ksw_watch_init(void);
+void ksw_watch_exit(void);
+
 #endif /* _KSTACKWATCH_H */
diff --git a/mm/kstackwatch/watch.c b/mm/kstackwatch/watch.c
index cec594032515..4947eac32c61 100644
--- a/mm/kstackwatch/watch.c
+++ b/mm/kstackwatch/watch.c
@@ -1 +1,94 @@
 // SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/cpuhotplug.h>
+#include <linux/hw_breakpoint.h>
+#include <linux/irqflags.h>
+#include <linux/kstackwatch.h>
+#include <linux/mutex.h>
+#include <linux/printk.h>
+
+static LLIST_HEAD(free_wp_list);
+static LIST_HEAD(all_wp_list);
+static DEFINE_MUTEX(all_wp_mutex);
+
+static ulong holder;
+
+static void ksw_watch_handler(struct perf_event *bp,
+			      struct perf_sample_data *data,
+			      struct pt_regs *regs)
+{
+	pr_err("========== KStackWatch: Caught stack corruption =======\n");
+	pr_err("config %s\n", ksw_get_config()->user_input);
+	dump_stack();
+	pr_err("=================== KStackWatch End ===================\n");
+
+	if (ksw_get_config()->panic_hit)
+		panic("Stack corruption detected");
+}
+
+static int ksw_watch_alloc(void)
+{
+	int max_watch = ksw_get_config()->max_watch;
+	struct ksw_watchpoint *wp;
+	int success = 0;
+	int ret;
+
+	init_llist_head(&free_wp_list);
+
+	//max_watch=0 means at most
+	while (!max_watch || success < max_watch) {
+		wp = kzalloc(sizeof(*wp), GFP_KERNEL);
+		if (!wp)
+			return success > 0 ? success : -EINVAL;
+
+		hw_breakpoint_init(&wp->attr);
+		wp->attr.bp_addr = (ulong)&holder;
+		wp->attr.bp_len = sizeof(ulong);
+		wp->attr.bp_type = HW_BREAKPOINT_W;
+		wp->event = register_wide_hw_breakpoint(&wp->attr,
+							ksw_watch_handler, wp);
+		if (IS_ERR((void *)wp->event)) {
+			ret = PTR_ERR((void *)wp->event);
+			kfree(wp);
+			return success > 0 ? success : ret;
+		}
+		llist_add(&wp->node, &free_wp_list);
+		mutex_lock(&all_wp_mutex);
+		list_add(&wp->list, &all_wp_list);
+		mutex_unlock(&all_wp_mutex);
+		success++;
+	}
+
+	return success;
+}
+
+static void ksw_watch_free(void)
+{
+	struct ksw_watchpoint *wp, *tmp;
+
+	mutex_lock(&all_wp_mutex);
+	list_for_each_entry_safe(wp, tmp, &all_wp_list, list) {
+		list_del(&wp->list);
+		unregister_wide_hw_breakpoint(wp->event);
+		kfree(wp);
+	}
+	mutex_unlock(&all_wp_mutex);
+}
+
+int ksw_watch_init(void)
+{
+	int ret;
+
+	ret = ksw_watch_alloc();
+	if (ret <= 0)
+		return -EBUSY;
+
+
+	return 0;
+}
+
+void ksw_watch_exit(void)
+{
+	ksw_watch_free();
+}
-- 
2.43.0


^ permalink raw reply related

* [PATCH v8 08/27] mm/ksw: Add atomic watchpoint management api
From: Jinchao Wang @ 2025-11-10 16:36 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

Add three functions for atomic lifecycle management of watchpoints:
- ksw_watch_get(): Acquires a watchpoint from a llist.
- ksw_watch_on(): Enables the watchpoint on all online CPUs.
- ksw_watch_off(): Disables the watchpoint and returns it to the llist.

For cross-CPU synchronization, updates are propagated using direct
modification on the local CPU and asynchronous IPIs for remote CPUs.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 include/linux/kstackwatch.h |  4 ++
 mm/kstackwatch/watch.c      | 85 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/include/linux/kstackwatch.h b/include/linux/kstackwatch.h
index eb9f2b4f2109..d7ea89c8c6af 100644
--- a/include/linux/kstackwatch.h
+++ b/include/linux/kstackwatch.h
@@ -44,11 +44,15 @@ const struct ksw_config *ksw_get_config(void);
 /* watch management */
 struct ksw_watchpoint {
 	struct perf_event *__percpu *event;
+	call_single_data_t __percpu *csd;
 	struct perf_event_attr attr;
 	struct llist_node node; // for atomic watch_on and off
 	struct list_head list; // for cpu online and offline
 };
 int ksw_watch_init(void);
 void ksw_watch_exit(void);
+int ksw_watch_get(struct ksw_watchpoint **out_wp);
+int ksw_watch_on(struct ksw_watchpoint *wp, ulong watch_addr, u16 watch_len);
+int ksw_watch_off(struct ksw_watchpoint *wp);
 
 #endif /* _KSTACKWATCH_H */
diff --git a/mm/kstackwatch/watch.c b/mm/kstackwatch/watch.c
index 4947eac32c61..3817a172dc25 100644
--- a/mm/kstackwatch/watch.c
+++ b/mm/kstackwatch/watch.c
@@ -27,11 +27,83 @@ static void ksw_watch_handler(struct perf_event *bp,
 		panic("Stack corruption detected");
 }
 
+static void ksw_watch_on_local_cpu(void *info)
+{
+	struct ksw_watchpoint *wp = info;
+	struct perf_event *bp;
+	ulong flags;
+	int cpu;
+	int ret;
+
+	local_irq_save(flags);
+	cpu = raw_smp_processor_id();
+	bp = per_cpu(*wp->event, cpu);
+	if (!bp) {
+		local_irq_restore(flags);
+		return;
+	}
+
+	ret = modify_wide_hw_breakpoint_local(bp, &wp->attr);
+	local_irq_restore(flags);
+	WARN(ret, "fail to reinstall HWBP on CPU%d ret %d", cpu, ret);
+}
+
+static void ksw_watch_update(struct ksw_watchpoint *wp, ulong addr, u16 len)
+{
+	call_single_data_t *csd;
+	int cur_cpu;
+	int cpu;
+
+	wp->attr.bp_addr = addr;
+	wp->attr.bp_len = len;
+
+	cur_cpu = raw_smp_processor_id();
+	for_each_online_cpu(cpu) {
+		/* remote cpu first */
+		if (cpu == cur_cpu)
+			continue;
+		csd = per_cpu_ptr(wp->csd, cpu);
+		smp_call_function_single_async(cpu, csd);
+	}
+	ksw_watch_on_local_cpu(wp);
+}
+
+int ksw_watch_get(struct ksw_watchpoint **out_wp)
+{
+	struct ksw_watchpoint *wp;
+	struct llist_node *node;
+
+	node = llist_del_first(&free_wp_list);
+	if (!node)
+		return -EBUSY;
+
+	wp = llist_entry(node, struct ksw_watchpoint, node);
+	WARN_ON_ONCE(wp->attr.bp_addr != (u64)&holder);
+
+	*out_wp = wp;
+	return 0;
+}
+int ksw_watch_on(struct ksw_watchpoint *wp, ulong watch_addr, u16 watch_len)
+{
+	ksw_watch_update(wp, watch_addr, watch_len);
+	return 0;
+}
+
+int ksw_watch_off(struct ksw_watchpoint *wp)
+{
+	WARN_ON_ONCE(wp->attr.bp_addr == (u64)&holder);
+	ksw_watch_update(wp, (ulong)&holder, sizeof(ulong));
+	llist_add(&wp->node, &free_wp_list);
+	return 0;
+}
+
 static int ksw_watch_alloc(void)
 {
 	int max_watch = ksw_get_config()->max_watch;
 	struct ksw_watchpoint *wp;
+	call_single_data_t *csd;
 	int success = 0;
+	int cpu;
 	int ret;
 
 	init_llist_head(&free_wp_list);
@@ -41,6 +113,16 @@ static int ksw_watch_alloc(void)
 		wp = kzalloc(sizeof(*wp), GFP_KERNEL);
 		if (!wp)
 			return success > 0 ? success : -EINVAL;
+		wp->csd = alloc_percpu(call_single_data_t);
+		if (!wp->csd) {
+			kfree(wp);
+			return success > 0 ? success : -EINVAL;
+		}
+
+		for_each_possible_cpu(cpu) {
+			csd = per_cpu_ptr(wp->csd, cpu);
+			INIT_CSD(csd, ksw_watch_on_local_cpu, wp);
+		}
 
 		hw_breakpoint_init(&wp->attr);
 		wp->attr.bp_addr = (ulong)&holder;
@@ -50,6 +132,7 @@ static int ksw_watch_alloc(void)
 							ksw_watch_handler, wp);
 		if (IS_ERR((void *)wp->event)) {
 			ret = PTR_ERR((void *)wp->event);
+			free_percpu(wp->csd);
 			kfree(wp);
 			return success > 0 ? success : ret;
 		}
@@ -71,6 +154,7 @@ static void ksw_watch_free(void)
 	list_for_each_entry_safe(wp, tmp, &all_wp_list, list) {
 		list_del(&wp->list);
 		unregister_wide_hw_breakpoint(wp->event);
+		free_percpu(wp->csd);
 		kfree(wp);
 	}
 	mutex_unlock(&all_wp_mutex);
@@ -84,7 +168,6 @@ int ksw_watch_init(void)
 	if (ret <= 0)
 		return -EBUSY;
 
-
 	return 0;
 }
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH v8 09/27] mm/ksw: ignore false positives from exit trampolines
From: Jinchao Wang @ 2025-11-10 16:36 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

Because trampolines run after the watched function returns but before the
exit_handler is called, and in the original stack frame, so the trampoline
code may overwrite the watched stack address.

These false positives should be ignored. is_ftrace_trampoline() does
not cover all trampolines, so add a local check to handle the remaining
cases.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/watch.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/mm/kstackwatch/watch.c b/mm/kstackwatch/watch.c
index 3817a172dc25..f922b4164be5 100644
--- a/mm/kstackwatch/watch.c
+++ b/mm/kstackwatch/watch.c
@@ -2,6 +2,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/cpuhotplug.h>
+#include <linux/ftrace.h>
 #include <linux/hw_breakpoint.h>
 #include <linux/irqflags.h>
 #include <linux/kstackwatch.h>
@@ -14,10 +15,46 @@ static DEFINE_MUTEX(all_wp_mutex);
 
 static ulong holder;
 
+#define TRAMPOLINE_NAME "return_to_handler"
+#define TRAMPOLINE_DEPTH 16
+
+/* Resolved once, then reused */
+static unsigned long tramp_start, tramp_end;
+
+static void ksw_watch_resolve_trampoline(void)
+{
+	unsigned long sz, off;
+
+	if (likely(tramp_start && tramp_end))
+		return;
+
+	tramp_start = kallsyms_lookup_name(TRAMPOLINE_NAME);
+	if (tramp_start && kallsyms_lookup_size_offset(tramp_start, &sz, &off))
+		tramp_end = tramp_start + sz;
+}
+
+static bool ksw_watch_in_trampoline(unsigned long ip)
+{
+	if (tramp_start && tramp_end && ip >= tramp_start && ip < tramp_end)
+		return true;
+	return false;
+}
 static void ksw_watch_handler(struct perf_event *bp,
 			      struct perf_sample_data *data,
 			      struct pt_regs *regs)
 {
+	unsigned long entries[TRAMPOLINE_DEPTH];
+	int i, nr = 0;
+
+	nr = stack_trace_save_regs(regs, entries, TRAMPOLINE_DEPTH, 0);
+	for (i = 0; i < nr; i++) {
+		//ignore trampoline
+		if (is_ftrace_trampoline(entries[i]))
+			return;
+		if (ksw_watch_in_trampoline(entries[i]))
+			return;
+	}
+
 	pr_err("========== KStackWatch: Caught stack corruption =======\n");
 	pr_err("config %s\n", ksw_get_config()->user_input);
 	dump_stack();
@@ -164,6 +201,7 @@ int ksw_watch_init(void)
 {
 	int ret;
 
+	ksw_watch_resolve_trampoline();
 	ret = ksw_watch_alloc();
 	if (ret <= 0)
 		return -EBUSY;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v8 10/27] mm/ksw: support CPU hotplug
From: Jinchao Wang @ 2025-11-10 16:36 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

Register CPU online/offline callbacks via cpuhp_setup_state_nocalls()
so stack watches are installed/removed dynamically as CPUs come online
or go offline.

When a new CPU comes online, register a hardware breakpoint for the holder,
avoiding races with watch_on()/watch_off() that may run on another CPU. The
watch address will be updated the next time watch_on() is called.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/watch.c | 52 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/mm/kstackwatch/watch.c b/mm/kstackwatch/watch.c
index f922b4164be5..99184f63d7e3 100644
--- a/mm/kstackwatch/watch.c
+++ b/mm/kstackwatch/watch.c
@@ -85,6 +85,48 @@ static void ksw_watch_on_local_cpu(void *info)
 	WARN(ret, "fail to reinstall HWBP on CPU%d ret %d", cpu, ret);
 }
 
+static int ksw_watch_cpu_online(unsigned int cpu)
+{
+	struct perf_event_attr attr;
+	struct ksw_watchpoint *wp;
+	call_single_data_t *csd;
+	struct perf_event *bp;
+
+	mutex_lock(&all_wp_mutex);
+	list_for_each_entry(wp, &all_wp_list, list) {
+		attr = wp->attr;
+		attr.bp_addr = (u64)&holder;
+		bp = perf_event_create_kernel_counter(&attr, cpu, NULL,
+						      ksw_watch_handler, wp);
+		if (IS_ERR(bp)) {
+			pr_warn("%s failed to create watch on CPU %d: %ld\n",
+				__func__, cpu, PTR_ERR(bp));
+			continue;
+		}
+
+		per_cpu(*wp->event, cpu) = bp;
+		csd = per_cpu_ptr(wp->csd, cpu);
+		INIT_CSD(csd, ksw_watch_on_local_cpu, wp);
+	}
+	mutex_unlock(&all_wp_mutex);
+	return 0;
+}
+
+static int ksw_watch_cpu_offline(unsigned int cpu)
+{
+	struct ksw_watchpoint *wp;
+	struct perf_event *bp;
+
+	mutex_lock(&all_wp_mutex);
+	list_for_each_entry(wp, &all_wp_list, list) {
+		bp = per_cpu(*wp->event, cpu);
+		if (bp)
+			unregister_hw_breakpoint(bp);
+	}
+	mutex_unlock(&all_wp_mutex);
+	return 0;
+}
+
 static void ksw_watch_update(struct ksw_watchpoint *wp, ulong addr, u16 len)
 {
 	call_single_data_t *csd;
@@ -206,6 +248,16 @@ int ksw_watch_init(void)
 	if (ret <= 0)
 		return -EBUSY;
 
+	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
+					"kstackwatch:online",
+					ksw_watch_cpu_online,
+					ksw_watch_cpu_offline);
+	if (ret < 0) {
+		ksw_watch_free();
+		pr_err("Failed to register CPU hotplug notifier\n");
+		return ret;
+	}
+
 	return 0;
 }
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH v8 11/27] sched/ksw: add per-task context
From: Jinchao Wang @ 2025-11-10 16:36 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

Introduce struct ksw_ctx to enable lockless per-task state
tracking. This is required because KStackWatch operates in NMI context
(via kprobe handler) where traditional locking is unsafe.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 include/linux/kstackwatch_types.h | 14 ++++++++++++++
 include/linux/sched.h             |  5 +++++
 2 files changed, 19 insertions(+)
 create mode 100644 include/linux/kstackwatch_types.h

diff --git a/include/linux/kstackwatch_types.h b/include/linux/kstackwatch_types.h
new file mode 100644
index 000000000000..8c4e9b0f0c6a
--- /dev/null
+++ b/include/linux/kstackwatch_types.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_KSTACKWATCH_TYPES_H
+#define _LINUX_KSTACKWATCH_TYPES_H
+#include <linux/types.h>
+
+struct ksw_watchpoint;
+struct ksw_ctx {
+	struct ksw_watchpoint *wp;
+	ulong sp;
+	u16 depth;
+	u16 generation;
+};
+
+#endif /* _LINUX_KSTACKWATCH_TYPES_H */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index b469878de25c..db49325428b3 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -22,6 +22,7 @@
 #include <linux/sem_types.h>
 #include <linux/shm.h>
 #include <linux/kmsan_types.h>
+#include <linux/kstackwatch_types.h>
 #include <linux/mutex_types.h>
 #include <linux/plist_types.h>
 #include <linux/hrtimer_types.h>
@@ -1487,6 +1488,10 @@ struct task_struct {
 	struct kmsan_ctx		kmsan_ctx;
 #endif
 
+#if IS_ENABLED(CONFIG_KSTACKWATCH)
+	struct ksw_ctx		ksw_ctx;
+#endif
+
 #if IS_ENABLED(CONFIG_KUNIT)
 	struct kunit			*kunit_test;
 #endif
-- 
2.43.0


^ permalink raw reply related

* [PATCH v8 12/27] mm/ksw: add entry kprobe and exit fprobe management
From: Jinchao Wang @ 2025-11-10 16:36 UTC (permalink / raw)
  To: Andrew Morton, Masami Hiramatsu (Google), Peter Zijlstra,
	Randy Dunlap, Marco Elver, Mike Rapoport, Alexander Potapenko,
	Adrian Hunter, Alexander Shishkin, Alice Ryhl, Andrey Konovalov,
	Andrey Ryabinin, Andrii Nakryiko, Ard Biesheuvel,
	Arnaldo Carvalho de Melo, Ben Segall, Bill Wendling,
	Borislav Petkov, Catalin Marinas, Dave Hansen, David Hildenbrand,
	David Kaplan, David S. Miller, Dietmar Eggemann, Dmitry Vyukov,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, James Clark,
	Jinchao Wang, Jinjie Ruan, Jiri Olsa, Jonathan Corbet, Juri Lelli,
	Justin Stitt, kasan-dev, Kees Cook, Liam R. Howlett, Liang Kan,
	Linus Walleij, linux-arm-kernel, linux-doc, linux-kernel,
	linux-mm, linux-perf-users, linux-trace-kernel, llvm,
	Lorenzo Stoakes, Mark Rutland, Masahiro Yamada, Mathieu Desnoyers,
	Mel Gorman, Michal Hocko, Miguel Ojeda, Nam Cao, Namhyung Kim,
	Nathan Chancellor, Naveen N Rao, Nick Desaulniers, Rong Xu,
	Sami Tolvanen, Steven Rostedt, Suren Baghdasaryan,
	Thomas Gleixner, Thomas Weißschuh, Valentin Schneider,
	Vincent Guittot, Vincenzo Frascino, Vlastimil Babka, Will Deacon,
	workflows, x86
In-Reply-To: <20251110163634.3686676-1-wangjinchao600@gmail.com>

Provide ksw_stack_init() and ksw_stack_exit() to manage entry and exit
probes for the target function from ksw_get_config().

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 include/linux/kstackwatch.h |   4 ++
 mm/kstackwatch/stack.c      | 100 ++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+)

diff --git a/include/linux/kstackwatch.h b/include/linux/kstackwatch.h
index d7ea89c8c6af..afedd9823de9 100644
--- a/include/linux/kstackwatch.h
+++ b/include/linux/kstackwatch.h
@@ -41,6 +41,10 @@ struct ksw_config {
 // singleton, only modified in kernel.c
 const struct ksw_config *ksw_get_config(void);
 
+/* stack management */
+int ksw_stack_init(void);
+void ksw_stack_exit(void);
+
 /* watch management */
 struct ksw_watchpoint {
 	struct perf_event *__percpu *event;
diff --git a/mm/kstackwatch/stack.c b/mm/kstackwatch/stack.c
index cec594032515..3aa02f8370af 100644
--- a/mm/kstackwatch/stack.c
+++ b/mm/kstackwatch/stack.c
@@ -1 +1,101 @@
 // SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/atomic.h>
+#include <linux/fprobe.h>
+#include <linux/kprobes.h>
+#include <linux/kstackwatch.h>
+#include <linux/kstackwatch_types.h>
+#include <linux/printk.h>
+
+static struct kprobe entry_probe;
+static struct fprobe exit_probe;
+
+static int ksw_stack_prepare_watch(struct pt_regs *regs,
+				   const struct ksw_config *config,
+				   ulong *watch_addr, u16 *watch_len)
+{
+	/* implement logic will be added in following patches */
+	*watch_addr = 0;
+	*watch_len = 0;
+	return 0;
+}
+
+static void ksw_stack_entry_handler(struct kprobe *p, struct pt_regs *regs,
+				    unsigned long flags)
+{
+	struct ksw_ctx *ctx = &current->ksw_ctx;
+	ulong watch_addr;
+	u16 watch_len;
+	int ret;
+
+	ret = ksw_watch_get(&ctx->wp);
+	if (ret)
+		return;
+
+	ret = ksw_stack_prepare_watch(regs, ksw_get_config(), &watch_addr,
+				      &watch_len);
+	if (ret) {
+		ksw_watch_off(ctx->wp);
+		ctx->wp = NULL;
+		pr_err("failed to prepare watch target: %d\n", ret);
+		return;
+	}
+
+	ret = ksw_watch_on(ctx->wp, watch_addr, watch_len);
+	if (ret) {
+		pr_err("failed to watch on depth:%d addr:0x%lx len:%u %d\n",
+		       ksw_get_config()->depth, watch_addr, watch_len, ret);
+		return;
+	}
+
+}
+
+static void ksw_stack_exit_handler(struct fprobe *fp, unsigned long ip,
+				   unsigned long ret_ip,
+				   struct ftrace_regs *regs, void *data)
+{
+	struct ksw_ctx *ctx = &current->ksw_ctx;
+
+
+	if (ctx->wp) {
+		ksw_watch_off(ctx->wp);
+		ctx->wp = NULL;
+		ctx->sp = 0;
+	}
+}
+
+int ksw_stack_init(void)
+{
+	int ret;
+	char *symbuf = NULL;
+
+	memset(&entry_probe, 0, sizeof(entry_probe));
+	entry_probe.symbol_name = ksw_get_config()->func_name;
+	entry_probe.offset = ksw_get_config()->func_offset;
+	entry_probe.post_handler = ksw_stack_entry_handler;
+	ret = register_kprobe(&entry_probe);
+	if (ret) {
+		pr_err("failed to register kprobe ret %d\n", ret);
+		return ret;
+	}
+
+	memset(&exit_probe, 0, sizeof(exit_probe));
+	exit_probe.exit_handler = ksw_stack_exit_handler;
+	symbuf = (char *)ksw_get_config()->func_name;
+
+	ret = register_fprobe_syms(&exit_probe, (const char **)&symbuf, 1);
+	if (ret < 0) {
+		pr_err("failed to register fprobe ret %d\n", ret);
+		unregister_kprobe(&entry_probe);
+		return ret;
+	}
+
+	return 0;
+}
+
+void ksw_stack_exit(void)
+{
+	unregister_fprobe(&exit_probe);
+	unregister_kprobe(&entry_probe);
+}
-- 
2.43.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox