* [for-next][PATCH 01/11] tracing: Make percpu stack trace buffer invariant to PAGE_SIZE
2024-11-01 10:36 [for-next][PATCH 00/11] tracing: Updates for 6.13 Steven Rostedt
@ 2024-11-01 10:36 ` Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 02/11] tracing: Replace multiple deprecated strncpy with memcpy Steven Rostedt
` (9 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 10:36 UTC (permalink / raw)
To: linux-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Ryan Roberts
From: Ryan Roberts <ryan.roberts@arm.com>
Previously the size of "struct ftrace_stacks" depended upon PAGE_SIZE.
For the common 4K page size, on a 64-bit system, sizeof(struct
ftrace_stacks) was 32K. But for a 64K page size, sizeof(struct
ftrace_stacks) was 512K.
But ftrace stack usage requirements should be invariant to page size. So
let's redefine FTRACE_KSTACK_ENTRIES so that "struct ftrace_stacks" is
always sized at 32K for 64-bit and 16K for 32-bit.
As a side effect, it removes the PAGE_SIZE compile-time constant
assumption from this code, which is required to reach the goal of
boot-time page size selection.
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://lore.kernel.org/20241021141832.3668264-1-ryan.roberts@arm.com
Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index bdb776e6ceb9..f1d613d924e9 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2898,7 +2898,7 @@ trace_function(struct trace_array *tr, unsigned long ip, unsigned long
/* Allow 4 levels of nesting: normal, softirq, irq, NMI */
#define FTRACE_KSTACK_NESTING 4
-#define FTRACE_KSTACK_ENTRIES (PAGE_SIZE / FTRACE_KSTACK_NESTING)
+#define FTRACE_KSTACK_ENTRIES (SZ_4K / FTRACE_KSTACK_NESTING)
struct ftrace_stack {
unsigned long calls[FTRACE_KSTACK_ENTRIES];
--
2.45.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 02/11] tracing: Replace multiple deprecated strncpy with memcpy
2024-11-01 10:36 [for-next][PATCH 00/11] tracing: Updates for 6.13 Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 01/11] tracing: Make percpu stack trace buffer invariant to PAGE_SIZE Steven Rostedt
@ 2024-11-01 10:36 ` Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 03/11] kdb: Replace the use of simple_strto with safer kstrto in kdb_main Steven Rostedt
` (8 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 10:36 UTC (permalink / raw)
To: linux-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
linux-hardening, Kees Cook, Justin Stitt
From: Justin Stitt <justinstitt@google.com>
strncpy() is deprecated for use on NUL-terminated destination strings [1] and
as such we should prefer more robust and less ambiguous string interfaces.
String copy operations involving manual pointer offset and length
calculations followed by explicit NUL-byte assignments are best changed
to either strscpy or memcpy.
strscpy is not a drop-in replacement as @len would need a one subtracted
from it to avoid truncating the source string.
To not sabotage readability of the current code, use memcpy (retaining
the manual NUL assignment) as this unambiguously describes the desired
behavior.
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://github.com/KSPP/linux/issues/90 [2]
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: linux-hardening@vger.kernel.org
Link: https://lore.kernel.org/20241014-strncpy-kernel-trace-trace_events_filter-c-v2-1-d821e81e371e@google.com
Reviewed-by: Kees Cook <kees@kernel.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_events_filter.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 0c611b281a5b..78051de581e7 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1616,7 +1616,7 @@ static int parse_pred(const char *str, void *data,
goto err_free;
}
- strncpy(num_buf, str + s, len);
+ memcpy(num_buf, str + s, len);
num_buf[len] = 0;
ret = kstrtoul(num_buf, 0, &ip);
@@ -1694,7 +1694,7 @@ static int parse_pred(const char *str, void *data,
if (!pred->regex)
goto err_mem;
pred->regex->len = len;
- strncpy(pred->regex->pattern, str + s, len);
+ memcpy(pred->regex->pattern, str + s, len);
pred->regex->pattern[len] = 0;
} else if (!strncmp(str + i, "CPUS", 4)) {
@@ -1859,7 +1859,7 @@ static int parse_pred(const char *str, void *data,
if (!pred->regex)
goto err_mem;
pred->regex->len = len;
- strncpy(pred->regex->pattern, str + s, len);
+ memcpy(pred->regex->pattern, str + s, len);
pred->regex->pattern[len] = 0;
filter_build_regex(pred);
@@ -1919,7 +1919,7 @@ static int parse_pred(const char *str, void *data,
goto err_free;
}
- strncpy(num_buf, str + s, len);
+ memcpy(num_buf, str + s, len);
num_buf[len] = 0;
/* Make sure it is a value */
--
2.45.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 03/11] kdb: Replace the use of simple_strto with safer kstrto in kdb_main
2024-11-01 10:36 [for-next][PATCH 00/11] tracing: Updates for 6.13 Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 01/11] tracing: Make percpu stack trace buffer invariant to PAGE_SIZE Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 02/11] tracing: Replace multiple deprecated strncpy with memcpy Steven Rostedt
@ 2024-11-01 10:36 ` Steven Rostedt
2024-11-01 14:21 ` Doug Anderson
2024-11-01 10:36 ` [for-next][PATCH 04/11] trace: kdb: Replace simple_strtoul with kstrtoul in kdb_ftdump Steven Rostedt
` (7 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 10:36 UTC (permalink / raw)
To: linux-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Yuran Pereira, Nir Lichtman, Douglas Anderson
From: Yuran Pereira <yuran.pereira@hotmail.com>
The simple_str* family of functions perform no error checking in
scenarios where the input value overflows the intended output variable.
This results in these functions successfully returning even when the
output does not match the input string.
Or as it was mentioned [1], "...simple_strtol(), simple_strtoll(),
simple_strtoul(), and simple_strtoull() functions explicitly ignore
overflows, which may lead to unexpected results in callers."
Hence, the use of those functions is discouraged.
This patch replaces all uses of the simple_strto* series of functions
with their safer kstrto* alternatives.
Side effects of this patch:
- Every string to long or long long conversion using kstrto* is now
checked for failure.
- kstrto* errors are handled with appropriate `KDB_BADINT` wherever
applicable.
- A good side effect is that we end up saving a few lines of code
since unlike in simple_strto* functions, kstrto functions do not
need an additional "end pointer" variable, and the return values
of the latter can be directly checked in an "if" statement without
the need to define additional `ret` or `err` variables.
This, of course, results in cleaner, yet still easy to understand
code.
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull
Link: https://lore.kernel.org/20241028191916.GA918454@lichtman.org
Signed-off-by: Yuran Pereira <yuran.pereira@hotmail.com>
[nir: addressed review comments by fixing styling, invalid conversion and a missing error return]
Signed-off-by: Nir Lichtman <nir@lichtman.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/debug/kdb/kdb_main.c | 69 +++++++++++--------------------------
1 file changed, 21 insertions(+), 48 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index f5f7d7fb5936..f8703ab760d9 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -306,8 +306,8 @@ static int kdbgetulenv(const char *match, unsigned long *value)
return KDB_NOTENV;
if (strlen(ep) == 0)
return KDB_NOENVVALUE;
-
- *value = simple_strtoul(ep, NULL, 0);
+ if (kstrtoul(ep, 0, value))
+ return KDB_BADINT;
return 0;
}
@@ -402,42 +402,23 @@ static void kdb_printenv(void)
*/
int kdbgetularg(const char *arg, unsigned long *value)
{
- char *endp;
- unsigned long val;
-
- val = simple_strtoul(arg, &endp, 0);
-
- if (endp == arg) {
- /*
- * Also try base 16, for us folks too lazy to type the
- * leading 0x...
- */
- val = simple_strtoul(arg, &endp, 16);
- if (endp == arg)
+ /*
+ * If the first fails, also try base 16, for us
+ * folks too lazy to type the leading 0x...
+ */
+ if (kstrtoul(arg, 0, value)) {
+ if (kstrtoul(arg, 16, value))
return KDB_BADINT;
}
-
- *value = val;
-
return 0;
}
int kdbgetu64arg(const char *arg, u64 *value)
{
- char *endp;
- u64 val;
-
- val = simple_strtoull(arg, &endp, 0);
-
- if (endp == arg) {
-
- val = simple_strtoull(arg, &endp, 16);
- if (endp == arg)
+ if (kstrtou64(arg, 0, value)) {
+ if (kstrtou64(arg, 16, value))
return KDB_BADINT;
}
-
- *value = val;
-
return 0;
}
@@ -473,10 +454,10 @@ int kdb_set(int argc, const char **argv)
*/
if (strcmp(argv[1], "KDBDEBUG") == 0) {
unsigned int debugflags;
- char *cp;
+ int ret;
- debugflags = simple_strtoul(argv[2], &cp, 0);
- if (cp == argv[2] || debugflags & ~KDB_DEBUG_FLAG_MASK) {
+ ret = kstrtouint(argv[2], 0, &debugflags);
+ if (ret || debugflags & ~KDB_DEBUG_FLAG_MASK) {
kdb_printf("kdb: illegal debug flags '%s'\n",
argv[2]);
return 0;
@@ -1619,10 +1600,10 @@ static int kdb_md(int argc, const char **argv)
if (!argv[0][3])
valid = 1;
else if (argv[0][3] == 'c' && argv[0][4]) {
- char *p;
- repeat = simple_strtoul(argv[0] + 4, &p, 10);
+ if (kstrtouint(argv[0] + 4, 10, &repeat))
+ return KDB_BADINT;
mdcount = ((repeat * bytesperword) + 15) / 16;
- valid = !*p;
+ valid = 1;
}
last_repeat = repeat;
} else if (strcmp(argv[0], "md") == 0)
@@ -2083,15 +2064,10 @@ static int kdb_dmesg(int argc, const char **argv)
if (argc > 2)
return KDB_ARGCOUNT;
if (argc) {
- char *cp;
- lines = simple_strtol(argv[1], &cp, 0);
- if (*cp)
+ if (kstrtoint(argv[1], 0, &lines))
lines = 0;
- if (argc > 1) {
- adjust = simple_strtoul(argv[2], &cp, 0);
- if (*cp || adjust < 0)
- adjust = 0;
- }
+ if (argc > 1 && (kstrtoint(argv[2], 0, &adjust) || adjust < 0))
+ adjust = 0;
}
/* disable LOGGING if set */
@@ -2428,14 +2404,12 @@ static int kdb_help(int argc, const char **argv)
static int kdb_kill(int argc, const char **argv)
{
long sig, pid;
- char *endp;
struct task_struct *p;
if (argc != 2)
return KDB_ARGCOUNT;
- sig = simple_strtol(argv[1], &endp, 0);
- if (*endp)
+ if (kstrtol(argv[1], 0, &sig))
return KDB_BADINT;
if ((sig >= 0) || !valid_signal(-sig)) {
kdb_printf("Invalid signal parameter.<-signal>\n");
@@ -2443,8 +2417,7 @@ static int kdb_kill(int argc, const char **argv)
}
sig = -sig;
- pid = simple_strtol(argv[2], &endp, 0);
- if (*endp)
+ if (kstrtol(argv[2], 0, &pid))
return KDB_BADINT;
if (pid <= 0) {
kdb_printf("Process ID must be large than 0.\n");
--
2.45.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [for-next][PATCH 03/11] kdb: Replace the use of simple_strto with safer kstrto in kdb_main
2024-11-01 10:36 ` [for-next][PATCH 03/11] kdb: Replace the use of simple_strto with safer kstrto in kdb_main Steven Rostedt
@ 2024-11-01 14:21 ` Doug Anderson
2024-11-01 14:31 ` Steven Rostedt
0 siblings, 1 reply; 19+ messages in thread
From: Doug Anderson @ 2024-11-01 14:21 UTC (permalink / raw)
To: Steven Rostedt, Daniel Thompson
Cc: linux-kernel, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
Andrew Morton, Yuran Pereira, Nir Lichtman
Hi,
On Fri, Nov 1, 2024 at 3:36 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: Yuran Pereira <yuran.pereira@hotmail.com>
>
> The simple_str* family of functions perform no error checking in
> scenarios where the input value overflows the intended output variable.
> This results in these functions successfully returning even when the
> output does not match the input string.
>
> Or as it was mentioned [1], "...simple_strtol(), simple_strtoll(),
> simple_strtoul(), and simple_strtoull() functions explicitly ignore
> overflows, which may lead to unexpected results in callers."
> Hence, the use of those functions is discouraged.
>
> This patch replaces all uses of the simple_strto* series of functions
> with their safer kstrto* alternatives.
>
> Side effects of this patch:
> - Every string to long or long long conversion using kstrto* is now
> checked for failure.
> - kstrto* errors are handled with appropriate `KDB_BADINT` wherever
> applicable.
> - A good side effect is that we end up saving a few lines of code
> since unlike in simple_strto* functions, kstrto functions do not
> need an additional "end pointer" variable, and the return values
> of the latter can be directly checked in an "if" statement without
> the need to define additional `ret` or `err` variables.
> This, of course, results in cleaner, yet still easy to understand
> code.
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull
>
> Link: https://lore.kernel.org/20241028191916.GA918454@lichtman.org
> Signed-off-by: Yuran Pereira <yuran.pereira@hotmail.com>
> [nir: addressed review comments by fixing styling, invalid conversion and a missing error return]
> Signed-off-by: Nir Lichtman <nir@lichtman.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> kernel/debug/kdb/kdb_main.c | 69 +++++++++++--------------------------
> 1 file changed, 21 insertions(+), 48 deletions(-)
FWIW, I personally have no objection to this patch and patch #3/3 in
Nir's series (#5/11 in your email thread) going through the ftrace
tree, I'm not actually the maintainer of kdb/kgdb. I'm a reviewer and
I try my best to help, but officially you should probably have Daniel
Thompson's Ack for them. ...or at least make sure he's CCed here
saying that you've picked them up.
I've added him to the conversation here.
-Doug
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [for-next][PATCH 03/11] kdb: Replace the use of simple_strto with safer kstrto in kdb_main
2024-11-01 14:21 ` Doug Anderson
@ 2024-11-01 14:31 ` Steven Rostedt
2024-11-01 18:22 ` Daniel Thompson
0 siblings, 1 reply; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 14:31 UTC (permalink / raw)
To: Doug Anderson
Cc: Daniel Thompson, linux-kernel, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Andrew Morton, Yuran Pereira, Nir Lichtman
On Fri, 1 Nov 2024 07:21:05 -0700
Doug Anderson <dianders@chromium.org> wrote:
> FWIW, I personally have no objection to this patch and patch #3/3 in
> Nir's series (#5/11 in your email thread) going through the ftrace
> tree, I'm not actually the maintainer of kdb/kgdb. I'm a reviewer and
> I try my best to help, but officially you should probably have Daniel
> Thompson's Ack for them. ...or at least make sure he's CCed here
> saying that you've picked them up.
>
> I've added him to the conversation here.
Sure, I can even drop this patch if need be. Thanks for adding Daniel to
the Cc. I probably should have run these patches through get maintainers to
make sure everyone was accounted for.
-- Steve
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [for-next][PATCH 03/11] kdb: Replace the use of simple_strto with safer kstrto in kdb_main
2024-11-01 14:31 ` Steven Rostedt
@ 2024-11-01 18:22 ` Daniel Thompson
2024-11-01 18:36 ` Steven Rostedt
2024-11-01 18:42 ` Nir Lichtman
0 siblings, 2 replies; 19+ messages in thread
From: Daniel Thompson @ 2024-11-01 18:22 UTC (permalink / raw)
To: Steven Rostedt
Cc: Doug Anderson, linux-kernel, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Andrew Morton, Yuran Pereira, Nir Lichtman
On Fri, Nov 01, 2024 at 10:31:28AM -0400, Steven Rostedt wrote:
> On Fri, 1 Nov 2024 07:21:05 -0700
> Doug Anderson <dianders@chromium.org> wrote:
>
> > FWIW, I personally have no objection to this patch and patch #3/3 in
> > Nir's series (#5/11 in your email thread) going through the ftrace
> > tree, I'm not actually the maintainer of kdb/kgdb. I'm a reviewer and
> > I try my best to help, but officially you should probably have Daniel
> > Thompson's Ack for them. ...or at least make sure he's CCed here
> > saying that you've picked them up.
> >
> > I've added him to the conversation here.
>
> Sure, I can even drop this patch if need be. Thanks for adding Daniel to
> the Cc. I probably should have run these patches through get maintainers to
> make sure everyone was accounted for.
I haven't had a chance to review or hoover up the kdb/kgdb patches for
this cycle yet (mixture of travel and other things) but they are queued
up in my TODO list for next week.
I presume the tracing tree is involved because one of them changes the
kdb ftrace command? Are there dependencies between that and other
patches in the seriesm?
Daniel.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [for-next][PATCH 03/11] kdb: Replace the use of simple_strto with safer kstrto in kdb_main
2024-11-01 18:22 ` Daniel Thompson
@ 2024-11-01 18:36 ` Steven Rostedt
2024-11-01 18:42 ` Nir Lichtman
1 sibling, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 18:36 UTC (permalink / raw)
To: Daniel Thompson
Cc: Doug Anderson, linux-kernel, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Andrew Morton, Yuran Pereira, Nir Lichtman
On Fri, 1 Nov 2024 18:22:04 +0000
Daniel Thompson <daniel.thompson@linaro.org> wrote:
> I presume the tracing tree is involved because one of them changes the
> kdb ftrace command? Are there dependencies between that and other
> patches in the seriesm?
Nope, I'll drop them from my queue.
You can add to the tracing patch:
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [for-next][PATCH 03/11] kdb: Replace the use of simple_strto with safer kstrto in kdb_main
2024-11-01 18:22 ` Daniel Thompson
2024-11-01 18:36 ` Steven Rostedt
@ 2024-11-01 18:42 ` Nir Lichtman
2024-11-01 18:54 ` Steven Rostedt
1 sibling, 1 reply; 19+ messages in thread
From: Nir Lichtman @ 2024-11-01 18:42 UTC (permalink / raw)
To: Daniel Thompson
Cc: Steven Rostedt, Doug Anderson, linux-kernel, Masami Hiramatsu,
Mark Rutland, Mathieu Desnoyers, Andrew Morton, Yuran Pereira
On Fri, Nov 01, 2024 at 06:22:04PM +0000, Daniel Thompson wrote:
> On Fri, Nov 01, 2024 at 10:31:28AM -0400, Steven Rostedt wrote:
> > On Fri, 1 Nov 2024 07:21:05 -0700
> > Doug Anderson <dianders@chromium.org> wrote:
> >
> > > FWIW, I personally have no objection to this patch and patch #3/3 in
> > > Nir's series (#5/11 in your email thread) going through the ftrace
> > > tree, I'm not actually the maintainer of kdb/kgdb. I'm a reviewer and
> > > I try my best to help, but officially you should probably have Daniel
> > > Thompson's Ack for them. ...or at least make sure he's CCed here
> > > saying that you've picked them up.
> > >
> > > I've added him to the conversation here.
> >
> > Sure, I can even drop this patch if need be. Thanks for adding Daniel to
> > the Cc. I probably should have run these patches through get maintainers to
> > make sure everyone was accounted for.
>
>
> I presume the tracing tree is involved because one of them changes the
> kdb ftrace command? Are there dependencies between that and other
> patches in the seriesm?
I assume that is the reason, I just used the same recipient list that
the original author of the patch series used a couple of months ago.
The patch series is mostly around migrating to usage of better string
to int conversion functions, so technically each change is not really
dependant on the others.
BTW, Thanks for the reviews Doug and for applying Steven.
Nir.
>
>
> Daniel.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [for-next][PATCH 03/11] kdb: Replace the use of simple_strto with safer kstrto in kdb_main
2024-11-01 18:42 ` Nir Lichtman
@ 2024-11-01 18:54 ` Steven Rostedt
2024-11-01 19:00 ` Nir Lichtman
0 siblings, 1 reply; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 18:54 UTC (permalink / raw)
To: Nir Lichtman
Cc: Daniel Thompson, Doug Anderson, linux-kernel, Masami Hiramatsu,
Mark Rutland, Mathieu Desnoyers, Andrew Morton, Yuran Pereira
On Fri, 1 Nov 2024 18:42:28 +0000
Nir Lichtman <nir@lichtman.org> wrote:
> I assume that is the reason, I just used the same recipient list that
> the original author of the patch series used a couple of months ago.
> The patch series is mostly around migrating to usage of better string
> to int conversion functions, so technically each change is not really
> dependant on the others.
>
> BTW, Thanks for the reviews Doug and for applying Steven.
Note, it's now in Daniel's court. I dropped the patches from linux-next.
I took Doug's Reviewed-by tags as saying it can go through my tree. But
that was jumping the gun.
Sorry for the confusion.
-- Steve
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [for-next][PATCH 03/11] kdb: Replace the use of simple_strto with safer kstrto in kdb_main
2024-11-01 18:54 ` Steven Rostedt
@ 2024-11-01 19:00 ` Nir Lichtman
0 siblings, 0 replies; 19+ messages in thread
From: Nir Lichtman @ 2024-11-01 19:00 UTC (permalink / raw)
To: Steven Rostedt
Cc: Daniel Thompson, Doug Anderson, linux-kernel, Masami Hiramatsu,
Mark Rutland, Mathieu Desnoyers, Andrew Morton, Yuran Pereira
On Fri, Nov 01, 2024 at 02:54:39PM -0400, Steven Rostedt wrote:
> On Fri, 1 Nov 2024 18:42:28 +0000
> Nir Lichtman <nir@lichtman.org> wrote:
>
> > BTW, Thanks for the reviews Doug and for applying Steven.
>
> Note, it's now in Daniel's court. I dropped the patches from linux-next.
>
> I took Doug's Reviewed-by tags as saying it can go through my tree. But
> that was jumping the gun.
>
> Sorry for the confusion.
Yah just saw that after sending the e-mail,
no worries, still thanks :)
Nir
>
> -- Steve
^ permalink raw reply [flat|nested] 19+ messages in thread
* [for-next][PATCH 04/11] trace: kdb: Replace simple_strtoul with kstrtoul in kdb_ftdump
2024-11-01 10:36 [for-next][PATCH 00/11] tracing: Updates for 6.13 Steven Rostedt
` (2 preceding siblings ...)
2024-11-01 10:36 ` [for-next][PATCH 03/11] kdb: Replace the use of simple_strto with safer kstrto in kdb_main Steven Rostedt
@ 2024-11-01 10:36 ` Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 05/11] kdb: Remove fallback interpretation of arbitrary numbers as hex Steven Rostedt
` (6 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 10:36 UTC (permalink / raw)
To: linux-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Yuran Pereira, Douglas Anderson, Nir Lichtman
From: Yuran Pereira <yuran.pereira@hotmail.com>
The function simple_strtoul performs no error checking in scenarios
where the input value overflows the intended output variable.
This results in this function successfully returning, even when the
output does not match the input string (aka the function returns
successfully even when the result is wrong).
Or as it was mentioned [1], "...simple_strtol(), simple_strtoll(),
simple_strtoul(), and simple_strtoull() functions explicitly ignore
overflows, which may lead to unexpected results in callers."
Hence, the use of those functions is discouraged.
This patch replaces all uses of the simple_strtoul with the safer
alternatives kstrtoint and kstrtol.
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull
Link: https://lore.kernel.org/20241028192100.GB918454@lichtman.org
Signed-off-by: Yuran Pereira <yuran.pereira@hotmail.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
[nir: style fixes]
Signed-off-by: Nir Lichtman <nir@lichtman.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_kdb.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c
index 59857a1ee44c..1e72d20b3c2f 100644
--- a/kernel/trace/trace_kdb.c
+++ b/kernel/trace/trace_kdb.c
@@ -96,22 +96,19 @@ static int kdb_ftdump(int argc, const char **argv)
{
int skip_entries = 0;
long cpu_file;
- char *cp;
+ int err;
int cnt;
int cpu;
if (argc > 2)
return KDB_ARGCOUNT;
- if (argc) {
- skip_entries = simple_strtol(argv[1], &cp, 0);
- if (*cp)
- skip_entries = 0;
- }
+ if (argc && kstrtoint(argv[1], 0, &skip_entries))
+ return KDB_BADINT;
if (argc == 2) {
- cpu_file = simple_strtol(argv[2], &cp, 0);
- if (*cp || cpu_file >= NR_CPUS || cpu_file < 0 ||
+ err = kstrtol(argv[2], 0, &cpu_file);
+ if (err || cpu_file >= NR_CPUS || cpu_file < 0 ||
!cpu_online(cpu_file))
return KDB_BADINT;
} else {
--
2.45.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 05/11] kdb: Remove fallback interpretation of arbitrary numbers as hex
2024-11-01 10:36 [for-next][PATCH 00/11] tracing: Updates for 6.13 Steven Rostedt
` (3 preceding siblings ...)
2024-11-01 10:36 ` [for-next][PATCH 04/11] trace: kdb: Replace simple_strtoul with kstrtoul in kdb_ftdump Steven Rostedt
@ 2024-11-01 10:36 ` Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 06/11] tracing: Remove TRACE_FLAG_IRQS_NOSUPPORT Steven Rostedt
` (5 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 10:36 UTC (permalink / raw)
To: linux-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Douglas Anderson, Nir Lichtman
From: Nir Lichtman <nir@lichtman.org>
Remove logic that enables a fallback of interpreting numbers supplied in KDB CLI
to be interpreted as hex without explicit "0x" prefix as this can be confusing
for the end users.
Link: https://lore.kernel.org/20241028192228.GC918454@lichtman.org
Suggested-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Nir Lichtman <nir@lichtman.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/debug/kdb/kdb_main.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index f8703ab760d9..5f4be507d79f 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -402,23 +402,15 @@ static void kdb_printenv(void)
*/
int kdbgetularg(const char *arg, unsigned long *value)
{
- /*
- * If the first fails, also try base 16, for us
- * folks too lazy to type the leading 0x...
- */
- if (kstrtoul(arg, 0, value)) {
- if (kstrtoul(arg, 16, value))
- return KDB_BADINT;
- }
+ if (kstrtoul(arg, 0, value))
+ return KDB_BADINT;
return 0;
}
int kdbgetu64arg(const char *arg, u64 *value)
{
- if (kstrtou64(arg, 0, value)) {
- if (kstrtou64(arg, 16, value))
- return KDB_BADINT;
- }
+ if (kstrtou64(arg, 0, value))
+ return KDB_BADINT;
return 0;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 06/11] tracing: Remove TRACE_FLAG_IRQS_NOSUPPORT
2024-11-01 10:36 [for-next][PATCH 00/11] tracing: Updates for 6.13 Steven Rostedt
` (4 preceding siblings ...)
2024-11-01 10:36 ` [for-next][PATCH 05/11] kdb: Remove fallback interpretation of arbitrary numbers as hex Steven Rostedt
@ 2024-11-01 10:36 ` Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 07/11] tracing: Introduce tracepoint extended structure Steven Rostedt
` (4 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 10:36 UTC (permalink / raw)
To: linux-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Peter Zijlstra, Sebastian Andrzej Siewior
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
It was possible to enable tracing with no IRQ tracing support. The
tracing infrastructure would then record TRACE_FLAG_IRQS_NOSUPPORT as
the only tracing flag and show an 'X' in the output.
The last user of this feature was PPC32 which managed to implement it
during PowerPC merge in 2009. Since then, it was unused and the PPC32
dependency was finally removed in commit 0ea5ee035133a ("tracing: Remove
PPC32 wart from config TRACING_SUPPORT").
Since the PowerPC merge the code behind !CONFIG_TRACE_IRQFLAGS_SUPPORT
with TRACING enabled can no longer be selected used and the 'X' is not
displayed or recorded.
Remove the CONFIG_TRACE_IRQFLAGS_SUPPORT from the tracing code. Remove
TRACE_FLAG_IRQS_NOSUPPORT.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://lore.kernel.org/20241022110112.XJI8I9T2@linutronix.de
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Documentation/trace/ftrace.rst | 3 ---
include/linux/trace_events.h | 13 -------------
kernel/trace/trace_output.c | 1 -
3 files changed, 17 deletions(-)
diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
index 4073ca48af4a..74d5bd801b1a 100644
--- a/Documentation/trace/ftrace.rst
+++ b/Documentation/trace/ftrace.rst
@@ -1031,9 +1031,6 @@ explains which is which.
CPU#: The CPU which the process was running on.
irqs-off: 'd' interrupts are disabled. '.' otherwise.
- .. caution:: If the architecture does not support a way to
- read the irq flags variable, an 'X' will always
- be printed here.
need-resched:
- 'N' both TIF_NEED_RESCHED and PREEMPT_NEED_RESCHED is set,
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index f8f2e52653df..016b29a56c87 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -184,7 +184,6 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status);
enum trace_flag_type {
TRACE_FLAG_IRQS_OFF = 0x01,
- TRACE_FLAG_IRQS_NOSUPPORT = 0x02,
TRACE_FLAG_NEED_RESCHED = 0x04,
TRACE_FLAG_HARDIRQ = 0x08,
TRACE_FLAG_SOFTIRQ = 0x10,
@@ -193,7 +192,6 @@ enum trace_flag_type {
TRACE_FLAG_BH_OFF = 0x80,
};
-#ifdef CONFIG_TRACE_IRQFLAGS_SUPPORT
static inline unsigned int tracing_gen_ctx_flags(unsigned long irqflags)
{
unsigned int irq_status = irqs_disabled_flags(irqflags) ?
@@ -207,17 +205,6 @@ static inline unsigned int tracing_gen_ctx(void)
local_save_flags(irqflags);
return tracing_gen_ctx_flags(irqflags);
}
-#else
-
-static inline unsigned int tracing_gen_ctx_flags(unsigned long irqflags)
-{
- return tracing_gen_ctx_irq_test(TRACE_FLAG_IRQS_NOSUPPORT);
-}
-static inline unsigned int tracing_gen_ctx(void)
-{
- return tracing_gen_ctx_irq_test(TRACE_FLAG_IRQS_NOSUPPORT);
-}
-#endif
static inline unsigned int tracing_gen_ctx_dec(void)
{
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 868f2f912f28..2ee6613dce6d 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -460,7 +460,6 @@ int trace_print_lat_fmt(struct trace_seq *s, struct trace_entry *entry)
(entry->flags & TRACE_FLAG_IRQS_OFF && bh_off) ? 'D' :
(entry->flags & TRACE_FLAG_IRQS_OFF) ? 'd' :
bh_off ? 'b' :
- (entry->flags & TRACE_FLAG_IRQS_NOSUPPORT) ? 'X' :
'.';
switch (entry->flags & (TRACE_FLAG_NEED_RESCHED |
--
2.45.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 07/11] tracing: Introduce tracepoint extended structure
2024-11-01 10:36 [for-next][PATCH 00/11] tracing: Updates for 6.13 Steven Rostedt
` (5 preceding siblings ...)
2024-11-01 10:36 ` [for-next][PATCH 06/11] tracing: Remove TRACE_FLAG_IRQS_NOSUPPORT Steven Rostedt
@ 2024-11-01 10:36 ` Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 08/11] tracing: Introduce tracepoint_is_faultable() Steven Rostedt
` (3 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 10:36 UTC (permalink / raw)
To: linux-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Jordan Rife, Michael Jeanson, Thomas Gleixner, Peter Zijlstra,
Alexei Starovoitov, Yonghong Song, Paul E. McKenney, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Namhyung Kim,
Andrii Nakryiko, bpf, Joel Fernandes, linux-trace-kernel
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Shrink the struct tracepoint size from 80 bytes to 72 bytes on x86-64 by
moving the (typically NULL) regfunc/unregfunc pointers to an extended
structure.
Tested-by: Jordan Rife <jrife@google.com>
Cc: Michael Jeanson <mjeanson@efficios.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Yonghong Song <yhs@fb.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: bpf@vger.kernel.org
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Jordan Rife <jrife@google.com>
Cc: linux-trace-kernel@vger.kernel.org
Link: https://lore.kernel.org/20241031152056.744137-2-mathieu.desnoyers@efficios.com
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/tracepoint-defs.h | 8 ++++++--
include/linux/tracepoint.h | 19 +++++++++++++------
kernel/tracepoint.c | 9 ++++-----
3 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index 60a6e8314d4c..967c08d9da84 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -29,6 +29,11 @@ struct tracepoint_func {
int prio;
};
+struct tracepoint_ext {
+ int (*regfunc)(void);
+ void (*unregfunc)(void);
+};
+
struct tracepoint {
const char *name; /* Tracepoint name */
struct static_key_false key;
@@ -36,9 +41,8 @@ struct tracepoint {
void *static_call_tramp;
void *iterator;
void *probestub;
- int (*regfunc)(void);
- void (*unregfunc)(void);
struct tracepoint_func __rcu *funcs;
+ struct tracepoint_ext *ext;
};
#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 0dc67fad706c..862ab49177a4 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -302,7 +302,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
* structures, so we create an array of pointers that will be used for iteration
* on the tracepoints.
*/
-#define DEFINE_TRACE_FN(_name, _reg, _unreg, proto, args) \
+#define __DEFINE_TRACE_EXT(_name, _ext, proto, args) \
static const char __tpstrtab_##_name[] \
__section("__tracepoints_strings") = #_name; \
extern struct static_call_key STATIC_CALL_KEY(tp_func_##_name); \
@@ -316,9 +316,9 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
.static_call_tramp = STATIC_CALL_TRAMP_ADDR(tp_func_##_name), \
.iterator = &__traceiter_##_name, \
.probestub = &__probestub_##_name, \
- .regfunc = _reg, \
- .unregfunc = _unreg, \
- .funcs = NULL }; \
+ .funcs = NULL, \
+ .ext = _ext, \
+ }; \
__TRACEPOINT_ENTRY(_name); \
int __traceiter_##_name(void *__data, proto) \
{ \
@@ -341,8 +341,15 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
} \
DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name);
-#define DEFINE_TRACE(name, proto, args) \
- DEFINE_TRACE_FN(name, NULL, NULL, PARAMS(proto), PARAMS(args));
+#define DEFINE_TRACE_FN(_name, _reg, _unreg, _proto, _args) \
+ static struct tracepoint_ext __tracepoint_ext_##_name = { \
+ .regfunc = _reg, \
+ .unregfunc = _unreg, \
+ }; \
+ __DEFINE_TRACE_EXT(_name, &__tracepoint_ext_##_name, PARAMS(_proto), PARAMS(_args));
+
+#define DEFINE_TRACE(_name, _proto, _args) \
+ __DEFINE_TRACE_EXT(_name, NULL, PARAMS(_proto), PARAMS(_args));
#define EXPORT_TRACEPOINT_SYMBOL_GPL(name) \
EXPORT_SYMBOL_GPL(__tracepoint_##name); \
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 6474e2cf22c9..5658dc92f5b5 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -278,8 +278,8 @@ static int tracepoint_add_func(struct tracepoint *tp,
struct tracepoint_func *old, *tp_funcs;
int ret;
- if (tp->regfunc && !static_key_enabled(&tp->key)) {
- ret = tp->regfunc();
+ if (tp->ext && tp->ext->regfunc && !static_key_enabled(&tp->key)) {
+ ret = tp->ext->regfunc();
if (ret < 0)
return ret;
}
@@ -362,9 +362,8 @@ static int tracepoint_remove_func(struct tracepoint *tp,
switch (nr_func_state(tp_funcs)) {
case TP_FUNC_0: /* 1->0 */
/* Removed last function */
- if (tp->unregfunc && static_key_enabled(&tp->key))
- tp->unregfunc();
-
+ if (tp->ext && tp->ext->unregfunc && static_key_enabled(&tp->key))
+ tp->ext->unregfunc();
static_branch_disable(&tp->key);
/* Set iterator static call */
tracepoint_update_call(tp, tp_funcs);
--
2.45.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 08/11] tracing: Introduce tracepoint_is_faultable()
2024-11-01 10:36 [for-next][PATCH 00/11] tracing: Updates for 6.13 Steven Rostedt
` (6 preceding siblings ...)
2024-11-01 10:36 ` [for-next][PATCH 07/11] tracing: Introduce tracepoint extended structure Steven Rostedt
@ 2024-11-01 10:36 ` Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 09/11] tracing: Fix syscall tracepoint use-after-free Steven Rostedt
` (2 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 10:36 UTC (permalink / raw)
To: linux-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Andrii Nakryiko, Jordan Rife, Michael Jeanson, Thomas Gleixner,
Peter Zijlstra, Alexei Starovoitov, Yonghong Song,
Paul E. McKenney, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Namhyung Kim, Andrii Nakryiko, bpf,
Joel Fernandes, linux-trace-kernel
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Introduce a "faultable" flag within the extended structure to know
whether a tracepoint needs rcu tasks trace grace period before reclaim.
This can be queried using tracepoint_is_faultable().
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Tested-by: Jordan Rife <jrife@google.com>
Cc: Michael Jeanson <mjeanson@efficios.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Yonghong Song <yhs@fb.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: bpf@vger.kernel.org
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Jordan Rife <jrife@google.com>
Cc: linux-trace-kernel@vger.kernel.org
Link: https://lore.kernel.org/20241031152056.744137-3-mathieu.desnoyers@efficios.com
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/tracepoint-defs.h | 2 ++
include/linux/tracepoint.h | 24 ++++++++++++++++++++++++
include/trace/define_trace.h | 2 +-
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index 967c08d9da84..aebf0571c736 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -32,6 +32,8 @@ struct tracepoint_func {
struct tracepoint_ext {
int (*regfunc)(void);
void (*unregfunc)(void);
+ /* Flags. */
+ unsigned int faultable:1;
};
struct tracepoint {
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 862ab49177a4..906f3091d23d 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -104,6 +104,12 @@ void for_each_tracepoint_in_module(struct module *mod,
* tracepoint_synchronize_unregister must be called between the last tracepoint
* probe unregistration and the end of module exit to make sure there is no
* caller executing a probe when it is freed.
+ *
+ * An alternative is to use the following for batch reclaim associated
+ * with a given tracepoint:
+ *
+ * - tracepoint_is_faultable() == false: call_rcu()
+ * - tracepoint_is_faultable() == true: call_rcu_tasks_trace()
*/
#ifdef CONFIG_TRACEPOINTS
static inline void tracepoint_synchronize_unregister(void)
@@ -111,9 +117,17 @@ static inline void tracepoint_synchronize_unregister(void)
synchronize_rcu_tasks_trace();
synchronize_rcu();
}
+static inline bool tracepoint_is_faultable(struct tracepoint *tp)
+{
+ return tp->ext && tp->ext->faultable;
+}
#else
static inline void tracepoint_synchronize_unregister(void)
{ }
+static inline bool tracepoint_is_faultable(struct tracepoint *tp)
+{
+ return false;
+}
#endif
#ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
@@ -345,6 +359,15 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
static struct tracepoint_ext __tracepoint_ext_##_name = { \
.regfunc = _reg, \
.unregfunc = _unreg, \
+ .faultable = false, \
+ }; \
+ __DEFINE_TRACE_EXT(_name, &__tracepoint_ext_##_name, PARAMS(_proto), PARAMS(_args));
+
+#define DEFINE_TRACE_SYSCALL(_name, _reg, _unreg, _proto, _args) \
+ static struct tracepoint_ext __tracepoint_ext_##_name = { \
+ .regfunc = _reg, \
+ .unregfunc = _unreg, \
+ .faultable = true, \
}; \
__DEFINE_TRACE_EXT(_name, &__tracepoint_ext_##_name, PARAMS(_proto), PARAMS(_args));
@@ -389,6 +412,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
#define __DECLARE_TRACE_SYSCALL __DECLARE_TRACE
#define DEFINE_TRACE_FN(name, reg, unreg, proto, args)
+#define DEFINE_TRACE_SYSCALL(name, reg, unreg, proto, args)
#define DEFINE_TRACE(name, proto, args)
#define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
#define EXPORT_TRACEPOINT_SYMBOL(name)
diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index ff5fa17a6259..63fea2218afa 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -48,7 +48,7 @@
#undef TRACE_EVENT_SYSCALL
#define TRACE_EVENT_SYSCALL(name, proto, args, struct, assign, print, reg, unreg) \
- DEFINE_TRACE_FN(name, reg, unreg, PARAMS(proto), PARAMS(args))
+ DEFINE_TRACE_SYSCALL(name, reg, unreg, PARAMS(proto), PARAMS(args))
#undef TRACE_EVENT_NOP
#define TRACE_EVENT_NOP(name, proto, args, struct, assign, print)
--
2.45.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 09/11] tracing: Fix syscall tracepoint use-after-free
2024-11-01 10:36 [for-next][PATCH 00/11] tracing: Updates for 6.13 Steven Rostedt
` (7 preceding siblings ...)
2024-11-01 10:36 ` [for-next][PATCH 08/11] tracing: Introduce tracepoint_is_faultable() Steven Rostedt
@ 2024-11-01 10:36 ` Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 10/11] tracing: Add might_fault() check in __DECLARE_TRACE_SYSCALL Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 11/11] tracing: Replace strncpy() with strscpy() when copying comm Steven Rostedt
10 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 10:36 UTC (permalink / raw)
To: linux-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
syzbot+b390c8062d8387b6272a, Jordan Rife, Michael Jeanson,
Thomas Gleixner, Peter Zijlstra, Alexei Starovoitov,
Yonghong Song, Paul E. McKenney, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Namhyung Kim,
Andrii Nakryiko, bpf, Joel Fernandes, linux-trace-kernel
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
The grace period used internally within tracepoint.c:release_probes()
uses call_rcu() to batch waiting for quiescence of old probe arrays,
rather than using the tracepoint_synchronize_unregister() which blocks
while waiting for quiescence.
With the introduction of faultable syscall tracepoints, this causes
use-after-free issues reproduced with syzkaller.
Fix this by using the appropriate call_rcu() or call_rcu_tasks_trace()
before invoking the rcu_free_old_probes callback. This can be chosen
using the tracepoint_is_faultable() API.
A similar issue exists in bpf use of call_rcu(). Fixing this is left to
a separate change.
Reported-by: syzbot+b390c8062d8387b6272a@syzkaller.appspotmail.com
Fixes: a363d27cdbc2 ("tracing: Allow system call tracepoints to handle page faults")
Tested-by: Jordan Rife <jrife@google.com>
Cc: Michael Jeanson <mjeanson@efficios.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Yonghong Song <yhs@fb.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: bpf@vger.kernel.org
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Jordan Rife <jrife@google.com>
Cc: linux-trace-kernel@vger.kernel.org
Link: https://lore.kernel.org/20241031152056.744137-4-mathieu.desnoyers@efficios.com
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/tracepoint.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 5658dc92f5b5..1848ce7e2976 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -106,13 +106,16 @@ static void rcu_free_old_probes(struct rcu_head *head)
kfree(container_of(head, struct tp_probes, rcu));
}
-static inline void release_probes(struct tracepoint_func *old)
+static inline void release_probes(struct tracepoint *tp, struct tracepoint_func *old)
{
if (old) {
struct tp_probes *tp_probes = container_of(old,
struct tp_probes, probes[0]);
- call_rcu(&tp_probes->rcu, rcu_free_old_probes);
+ if (tracepoint_is_faultable(tp))
+ call_rcu_tasks_trace(&tp_probes->rcu, rcu_free_old_probes);
+ else
+ call_rcu(&tp_probes->rcu, rcu_free_old_probes);
}
}
@@ -334,7 +337,7 @@ static int tracepoint_add_func(struct tracepoint *tp,
break;
}
- release_probes(old);
+ release_probes(tp, old);
return 0;
}
@@ -405,7 +408,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
WARN_ON_ONCE(1);
break;
}
- release_probes(old);
+ release_probes(tp, old);
return 0;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 10/11] tracing: Add might_fault() check in __DECLARE_TRACE_SYSCALL
2024-11-01 10:36 [for-next][PATCH 00/11] tracing: Updates for 6.13 Steven Rostedt
` (8 preceding siblings ...)
2024-11-01 10:36 ` [for-next][PATCH 09/11] tracing: Fix syscall tracepoint use-after-free Steven Rostedt
@ 2024-11-01 10:36 ` Steven Rostedt
2024-11-01 10:36 ` [for-next][PATCH 11/11] tracing: Replace strncpy() with strscpy() when copying comm Steven Rostedt
10 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 10:36 UTC (permalink / raw)
To: linux-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Thomas Gleixner, Jordan Rife, Michael Jeanson, Peter Zijlstra,
Alexei Starovoitov, Yonghong Song, Paul E. McKenney, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Namhyung Kim,
Andrii Nakryiko, bpf, Joel Fernandes, linux-trace-kernel
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Catch incorrect use of syscall tracepoints even if no probes are
registered by adding a might_fault() check in trace_##name()
emitted by __DECLARE_TRACE_SYSCALL.
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Jordan Rife <jrife@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Michael Jeanson <mjeanson@efficios.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Yonghong Song <yhs@fb.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: bpf@vger.kernel.org
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Jordan Rife <jrife@google.com>
Cc: linux-trace-kernel@vger.kernel.org
Link: https://lore.kernel.org/20241031152056.744137-5-mathieu.desnoyers@efficios.com
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/tracepoint.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 906f3091d23d..425123e921ac 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -301,6 +301,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
__DECLARE_TRACE_COMMON(name, PARAMS(proto), PARAMS(args), cond, PARAMS(data_proto)) \
static inline void trace_##name(proto) \
{ \
+ might_fault(); \
if (static_branch_unlikely(&__tracepoint_##name.key)) \
__DO_TRACE(name, \
TP_ARGS(args), \
--
2.45.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [for-next][PATCH 11/11] tracing: Replace strncpy() with strscpy() when copying comm
2024-11-01 10:36 [for-next][PATCH 00/11] tracing: Updates for 6.13 Steven Rostedt
` (9 preceding siblings ...)
2024-11-01 10:36 ` [for-next][PATCH 10/11] tracing: Add might_fault() check in __DECLARE_TRACE_SYSCALL Steven Rostedt
@ 2024-11-01 10:36 ` Steven Rostedt
10 siblings, 0 replies; 19+ messages in thread
From: Steven Rostedt @ 2024-11-01 10:36 UTC (permalink / raw)
To: linux-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Jinjie Ruan
From: Jinjie Ruan <ruanjinjie@huawei.com>
Replace the depreciated[1] strncpy() calls with strscpy()
when copying comm.
Link: https://github.com/KSPP/linux/issues/90 [1]
Cc: <mhiramat@kernel.org>
Cc: <mathieu.desnoyers@efficios.com>
Link: https://lore.kernel.org/20241031120139.1343025-1-ruanjinjie@huawei.com
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.c | 2 +-
kernel/trace/trace_events_hist.c | 4 ++--
kernel/trace/trace_sched_switch.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index f1d613d924e9..a587fd7d7447 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1921,7 +1921,7 @@ __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
max_data->critical_start = data->critical_start;
max_data->critical_end = data->critical_end;
- strncpy(max_data->comm, tsk->comm, TASK_COMM_LEN);
+ strscpy(max_data->comm, tsk->comm);
max_data->pid = tsk->pid;
/*
* If tsk == current, then use current_uid(), as that does not use
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index cc2924ad32a3..c288b92fc4df 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1599,7 +1599,7 @@ static inline void save_comm(char *comm, struct task_struct *task)
return;
}
- strncpy(comm, task->comm, TASK_COMM_LEN);
+ strscpy(comm, task->comm, TASK_COMM_LEN);
}
static void hist_elt_data_free(struct hist_elt_data *elt_data)
@@ -3405,7 +3405,7 @@ static bool cond_snapshot_update(struct trace_array *tr, void *cond_data)
elt_data = context->elt->private_data;
track_elt_data = track_data->elt.private_data;
if (elt_data->comm)
- strncpy(track_elt_data->comm, elt_data->comm, TASK_COMM_LEN);
+ strscpy(track_elt_data->comm, elt_data->comm, TASK_COMM_LEN);
track_data->updated = true;
diff --git a/kernel/trace/trace_sched_switch.c b/kernel/trace/trace_sched_switch.c
index 8a407adb0e1c..573b5d8e8a28 100644
--- a/kernel/trace/trace_sched_switch.c
+++ b/kernel/trace/trace_sched_switch.c
@@ -187,7 +187,7 @@ static inline char *get_saved_cmdlines(int idx)
static inline void set_cmdline(int idx, const char *cmdline)
{
- strncpy(get_saved_cmdlines(idx), cmdline, TASK_COMM_LEN);
+ strscpy(get_saved_cmdlines(idx), cmdline, TASK_COMM_LEN);
}
static void free_saved_cmdlines_buffer(struct saved_cmdlines_buffer *s)
--
2.45.2
^ permalink raw reply related [flat|nested] 19+ messages in thread