| Message ID | 20250404135850.2695211-1-devaanshk840@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] tracing: Replace deprecated strncpy() with strscpy() for stack_trace_filter_buf | expand |
On Fri, 4 Apr 2025 19:28:48 +0530 Devaansh Kumar <devaanshk840@gmail.com> wrote: > --- a/kernel/trace/trace_stack.c > +++ b/kernel/trace/trace_stack.c > @@ -543,8 +543,10 @@ static __init int enable_stacktrace(char *str) > { > int len; > > - if ((len = str_has_prefix(str, "_filter="))) > - strncpy(stack_trace_filter_buf, str + len, COMMAND_LINE_SIZE); > + len = str_has_prefix(str, "_filter="); > + > + if (len) > + strscpy(stack_trace_filter_buf, str + len, sizeof(stack_trace_filter_buf)); > > stack_tracer_enabled = 1; > return 1; BTW, why separate out the len? I use this notation quite a bit for str_has_prefix() checks. $ git grep '(len = str_has_pref' kernel/trace kernel/trace/trace_events.c: if (!(len = str_has_prefix(fmt, "REC->"))) kernel/trace/trace_events_filter.c: if ((len = str_has_prefix(str + i, ".ustring"))) { kernel/trace/trace_events_filter.c: if ((len = str_has_prefix(str + i, ".function"))) { kernel/trace/trace_events_hist.c: if ((len = str_has_prefix(str, "key=")) || kernel/trace/trace_events_hist.c: (len = str_has_prefix(str, "keys="))) { kernel/trace/trace_events_hist.c: } else if ((len = str_has_prefix(str, "val=")) || kernel/trace/trace_events_hist.c: (len = str_has_prefix(str, "vals=")) || kernel/trace/trace_events_hist.c: (len = str_has_prefix(str, "values="))) { kernel/trace/trace_events_hist.c: } else if ((len = str_has_prefix(str, "sort="))) { kernel/trace/trace_events_hist.c: } else if ((len = str_has_prefix(str, "clock="))) { kernel/trace/trace_events_hist.c: } else if ((len = str_has_prefix(str, "size="))) { kernel/trace/trace_events_hist.c: if ((len = str_has_prefix(str, "onmatch("))) kernel/trace/trace_events_hist.c: else if ((len = str_has_prefix(str, "onmax("))) kernel/trace/trace_events_hist.c: else if ((len = str_has_prefix(str, "onchange("))) kernel/trace/trace_stack.c: if ((len = str_has_prefix(str, "_filter="))) It's fine being in the if statement as the if is more about "does this have this prefix" and the length is just a side effect of it to use it to extract the rest. This patch is only about removing strncpy(), it doesn't need to modify the format. -- Steve
On Fri, 4 Apr 2025 at 23:49, Steven Rostedt <rostedt@goodmis.org> wrote: > > On Fri, 4 Apr 2025 19:28:48 +0530 > Devaansh Kumar <devaanshk840@gmail.com> wrote: > > > --- a/kernel/trace/trace_stack.c > > +++ b/kernel/trace/trace_stack.c > > @@ -543,8 +543,10 @@ static __init int enable_stacktrace(char *str) > > { > > int len; > > > > - if ((len = str_has_prefix(str, "_filter="))) > > - strncpy(stack_trace_filter_buf, str + len, COMMAND_LINE_SIZE); > > + len = str_has_prefix(str, "_filter="); > > + > > + if (len) > > + strscpy(stack_trace_filter_buf, str + len, sizeof(stack_trace_filter_buf)); > > > > stack_tracer_enabled = 1; > > return 1; > > BTW, why separate out the len? I use this notation quite a bit for > str_has_prefix() checks. > > $ git grep '(len = str_has_pref' kernel/trace > kernel/trace/trace_events.c: if (!(len = str_has_prefix(fmt, "REC->"))) > kernel/trace/trace_events_filter.c: if ((len = str_has_prefix(str + i, ".ustring"))) { > kernel/trace/trace_events_filter.c: if ((len = str_has_prefix(str + i, ".function"))) { > kernel/trace/trace_events_hist.c: if ((len = str_has_prefix(str, "key=")) || > kernel/trace/trace_events_hist.c: (len = str_has_prefix(str, "keys="))) { > kernel/trace/trace_events_hist.c: } else if ((len = str_has_prefix(str, "val=")) || > kernel/trace/trace_events_hist.c: (len = str_has_prefix(str, "vals=")) || > kernel/trace/trace_events_hist.c: (len = str_has_prefix(str, "values="))) { > kernel/trace/trace_events_hist.c: } else if ((len = str_has_prefix(str, "sort="))) { > kernel/trace/trace_events_hist.c: } else if ((len = str_has_prefix(str, "clock="))) { > kernel/trace/trace_events_hist.c: } else if ((len = str_has_prefix(str, "size="))) { > kernel/trace/trace_events_hist.c: if ((len = str_has_prefix(str, "onmatch("))) > kernel/trace/trace_events_hist.c: else if ((len = str_has_prefix(str, "onmax("))) > kernel/trace/trace_events_hist.c: else if ((len = str_has_prefix(str, "onchange("))) > kernel/trace/trace_stack.c: if ((len = str_has_prefix(str, "_filter="))) > > It's fine being in the if statement as the if is more about "does this have > this prefix" and the length is just a side effect of it to use it to > extract the rest. > > This patch is only about removing strncpy(), it doesn't need to modify the > format. > > -- Steve Sure I'll keep that in mind. -- Devaansh Kumar
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c index 5a48dba912ea..91ed2b76fa4c 100644 --- a/kernel/trace/trace_stack.c +++ b/kernel/trace/trace_stack.c @@ -543,8 +543,10 @@ static __init int enable_stacktrace(char *str) { int len; - if ((len = str_has_prefix(str, "_filter="))) - strncpy(stack_trace_filter_buf, str + len, COMMAND_LINE_SIZE); + len = str_has_prefix(str, "_filter="); + + if (len) + strscpy(stack_trace_filter_buf, str + len, sizeof(stack_trace_filter_buf)); stack_tracer_enabled = 1; return 1;
strncpy() is deprecated for NUL-terminated destination buffers and must be replaced by strscpy(). See issue: https://github.com/KSPP/linux/issues/90 Signed-off-by: Devaansh Kumar <devaanshk840@gmail.com> --- kernel/trace/trace_stack.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)