[PATCH v2 0/2] tracing: Misc bugfixes

[PATCH v2 0/2] tracing: Misc bugfixes

[Tom Zanussi]

There were 2 patches in v1 that were flagged by the 0day robot for
strncpy off-by-1 warnings:

  tracing: Remove size restriction on hist trigger cmd error logging
  tracing: Remove size restriction on synthetic event cmd error logging

  warning: 'strncpy' output truncated before terminating nul copying
  as many bytes from a string as its length [-Wstringop-truncation]

This v2 patchset consists of just those two patches with the fix for
those warnings.

Tom

v1 text below


Hi Steve,

This is an assorted bunch of bugfixes addressing a bugzilla bug,
smatch warnings, and related things I found while fixing those.  In
particular, when looking at the bugzilla bug,
https://bugzilla.kernel.org/show_bug.cgi?id=215513, I noticed that the
err_log output looked truncated and looking further into it found that
it was, because the hist trigger command was very long, exceeding the
256 character limit.

Obviously that's too short, and the final 3 patches remove that
limitation.

Tom

The following changes since commit ca965f23256b350ebd94b3dc1a319f28e8267f5f:

  tracing: Remove size restriction on tracing_log_err cmd strings (2022-01-27 15:38:28 -0600)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-trace.git ftrace/misc-bugfixes-v2

Tom Zanussi (2):
  tracing: Remove size restriction on hist trigger cmd error logging
  tracing: Remove size restriction on synthetic event cmd error logging

 kernel/trace/trace_events_hist.c  | 30 +++++++++++++++++++++++-------
 kernel/trace/trace_events_synth.c | 17 ++++++++++++++---
 2 files changed, 37 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH v2 1/2] tracing: Remove size restriction on hist trigger cmd error logging

[Tom Zanussi]

Currently, hist trigger command error strings are restricted to a
length of MAX_FILTER_STR_VAL (256), which is too short for some
commands already seen in the wild (with cmd strings longer than that
showing up truncated in err_log).

Remove the restriction so that no hist trigger command error string is
ever truncated.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 kernel/trace/trace_events_hist.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index ada87bfb5bb8..5e8970624bce 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -727,11 +727,16 @@ static struct track_data *track_data_alloc(unsigned int key_len,
 	return data;
 }
 
-static char last_cmd[MAX_FILTER_STR_VAL];
+#define HIST_PREFIX "hist:"
+
+static char *last_cmd;
 static char last_cmd_loc[MAX_FILTER_STR_VAL];
 
 static int errpos(char *str)
 {
+	if (!str || !last_cmd)
+		return 0;
+
 	return err_pos(last_cmd, str);
 }
 
@@ -739,12 +744,19 @@ static void last_cmd_set(struct trace_event_file *file, char *str)
 {
 	const char *system = NULL, *name = NULL;
 	struct trace_event_call *call;
+	int len = 0;
 
 	if (!str)
 		return;
 
-	strcpy(last_cmd, "hist:");
-	strncat(last_cmd, str, MAX_FILTER_STR_VAL - 1 - sizeof("hist:"));
+	len += sizeof(HIST_PREFIX) + strlen(str) + 1;
+	kfree(last_cmd);
+	last_cmd = kzalloc(len, GFP_KERNEL);
+	if (!last_cmd)
+		return;
+
+	strcpy(last_cmd, HIST_PREFIX);
+	strncat(last_cmd, str, len - sizeof(HIST_PREFIX));
 
 	if (file) {
 		call = file->event_call;
@@ -757,18 +769,22 @@ static void last_cmd_set(struct trace_event_file *file, char *str)
 	}
 
 	if (system)
-		snprintf(last_cmd_loc, MAX_FILTER_STR_VAL, "hist:%s:%s", system, name);
+		snprintf(last_cmd_loc, MAX_FILTER_STR_VAL, HIST_PREFIX "%s:%s", system, name);
 }
 
-static void hist_err(struct trace_array *tr, u8 err_type, u8 err_pos)
+static void hist_err(struct trace_array *tr, u8 err_type, u16 err_pos)
 {
+	if (!last_cmd)
+		return;
+
 	tracing_log_err(tr, last_cmd_loc, last_cmd, err_text,
 			err_type, err_pos);
 }
 
 static void hist_err_clear(void)
 {
-	last_cmd[0] = '\0';
+	if (last_cmd)
+		last_cmd[0] = '\0';
 	last_cmd_loc[0] = '\0';
 }
 
@@ -5610,7 +5626,7 @@ static int event_hist_trigger_print(struct seq_file *m,
 	bool have_var = false;
 	unsigned int i;
 
-	seq_puts(m, "hist:");
+	seq_puts(m, HIST_PREFIX);
 
 	if (data->name)
 		seq_printf(m, "%s:", data->name);
-- 
2.17.1

[PATCH v2 2/2] tracing: Remove size restriction on synthetic event cmd error logging

[Tom Zanussi]

Currently, synthetic event command error strings are restricted to a
length of MAX_FILTER_STR_VAL (256), which is too short for some
commands already seen in the wild (with cmd strings longer than that
showing up truncated in err_log).

Remove the restriction so that no synthetic event command error string
is ever truncated.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
 kernel/trace/trace_events_synth.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
index 149011e34ad9..d3d9cd677f9a 100644
--- a/kernel/trace/trace_events_synth.c
+++ b/kernel/trace/trace_events_synth.c
@@ -42,10 +42,13 @@ enum { ERRORS };
 
 static const char *err_text[] = { ERRORS };
 
-static char last_cmd[MAX_FILTER_STR_VAL];
+static char *last_cmd;
 
 static int errpos(const char *str)
 {
+	if (!str || !last_cmd)
+		return 0;
+
 	return err_pos(last_cmd, str);
 }
 
@@ -54,11 +57,19 @@ static void last_cmd_set(const char *str)
 	if (!str)
 		return;
 
-	strncpy(last_cmd, str, MAX_FILTER_STR_VAL - 1);
+	kfree(last_cmd);
+	last_cmd = kzalloc(strlen(str) + 1, GFP_KERNEL);
+	if (!last_cmd)
+		return;
+
+	strncpy(last_cmd, str, strlen(str) + 1);
 }
 
-static void synth_err(u8 err_type, u8 err_pos)
+static void synth_err(u8 err_type, u16 err_pos)
 {
+	if (!last_cmd)
+		return;
+
 	tracing_log_err(NULL, "synthetic_events", last_cmd, err_text,
 			err_type, err_pos);
 }
-- 
2.17.1

Re: [PATCH v2 0/2] tracing: Misc bugfixes

[Steven Rostedt]

On Fri, 28 Jan 2022 14:08:25 -0600
Tom Zanussi <zanussi@kernel.org> wrote:

> There were 2 patches in v1 that were flagged by the 0day robot for
> strncpy off-by-1 warnings:
> 
>   tracing: Remove size restriction on hist trigger cmd error logging
>   tracing: Remove size restriction on synthetic event cmd error logging
> 
>   warning: 'strncpy' output truncated before terminating nul copying
>   as many bytes from a string as its length [-Wstringop-truncation]
> 
> This v2 patchset consists of just those two patches with the fix for
> those warnings.


So this just replaces the last two patches you sent previously?

Probably would have been better to send all three (since the other 4
already made it into mainline).

But if "tracing: Remove size restriction on tracing_log_err cmd strings" is
OK:

   https://lkml.kernel.org/r/ca965f23256b350ebd94b3dc1a319f28e8267f5f.1643319703.git.zanussi@kernel.org

then I'll just replace the other two.

-- Steve

Re: [PATCH v2 0/2] tracing: Misc bugfixes

[Tom Zanussi]

On Fri, 2022-01-28 at 16:42 -0500, Steven Rostedt wrote:
> On Fri, 28 Jan 2022 14:08:25 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
> 
> > There were 2 patches in v1 that were flagged by the 0day robot for
> > strncpy off-by-1 warnings:
> > 
> >   tracing: Remove size restriction on hist trigger cmd error
> > logging
> >   tracing: Remove size restriction on synthetic event cmd error
> > logging
> > 
> >   warning: 'strncpy' output truncated before terminating nul
> > copying
> >   as many bytes from a string as its length [-Wstringop-truncation]
> > 
> > This v2 patchset consists of just those two patches with the fix
> > for
> > those warnings.
> 
> 
> So this just replaces the last two patches you sent previously?
> 

Yes.

> Probably would have been better to send all three (since the other 4
> already made it into mainline).
> 

OK, yeah, thought about doing that but guessed wrong, as usual ;-)

> But if "tracing: Remove size restriction on tracing_log_err cmd
> strings" is
> OK:
> 
>    

Yeah, that one is ok as-is without changes.

Tom

> 
https://lkml.kernel.org/r/ca965f23256b350ebd94b3dc1a319f28e8267f5f.1643319703.git.zanussi@kernel.org
> 
> then I'll just replace the other two.
> 
> -- Steve