> On May 13, 2021, at 3:17 PM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > > On Thu, 13 May 2021 19:08:13 +0000 > Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote: > >> The tracepoints that currently use '%.*s' no longer work when >> using "trace-cmd start/stop/show". They were working before >> 9a6944fee68e, so I consider this a regression. I plan to >> submit patches to address this for 5.13-rc. I guess they will >> have to go without the use of the new _len macros for now, >> and you can push the macros in v5.14. > > That's a separate bug. I'm currently running this patch through my tests, > and will push to Linus when it completes. Feel free to test this one too. Confirmed that applying the below patch addresses the regression. Tested-by: Chuck Lever <chuck.lever@xxxxxxxxxx> Thanks for your quick response! > -- Steve > > From eb01f5353bdaa59600b29d864819056a0e3de24d Mon Sep 17 00:00:00 2001 > From: "Steven Rostedt (VMware)" <rostedt@xxxxxxxxxxx> > Date: Thu, 13 May 2021 12:23:24 -0400 > Subject: [PATCH] tracing: Handle %.*s in trace_check_vprintf() > > If a trace event uses the %*.s notation, the trace_check_vprintf() will > fail and will warn about a bad processing of strings, because it does not > take into account the length field when processing the star (*) part. > Have it handle this case as well. > > Link: https://lore.kernel.org/linux-nfs/238C0E2D-C2A4-4578-ADD2-C565B3B99842@xxxxxxxxxx/ > > Reported-by: Chuck Lever III <chuck.lever@xxxxxxxxxx> > Fixes: 9a6944fee68e2 ("tracing: Add a verifier to check string pointers for trace events") > Signed-off-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx> > --- > kernel/trace/trace.c | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index 560e4c8d3825..a21ef9cd2aae 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -3704,6 +3704,9 @@ void trace_check_vprintf(struct trace_iterator *iter, const char *fmt, > goto print; > > while (*p) { > + bool star = false; > + int len = 0; > + > j = 0; > > /* We only care about %s and variants */ > @@ -3725,13 +3728,17 @@ void trace_check_vprintf(struct trace_iterator *iter, const char *fmt, > /* Need to test cases like %08.*s */ > for (j = 1; p[i+j]; j++) { > if (isdigit(p[i+j]) || > - p[i+j] == '*' || > p[i+j] == '.') > continue; > + if (p[i+j] == '*') { > + star = true; > + continue; > + } > break; > } > if (p[i+j] == 's') > break; > + star = false; > } > j = 0; > } > @@ -3744,6 +3751,9 @@ void trace_check_vprintf(struct trace_iterator *iter, const char *fmt, > iter->fmt[i] = '\0'; > trace_seq_vprintf(&iter->seq, iter->fmt, ap); > > + if (star) > + len = va_arg(ap, int); > + > /* The ap now points to the string data of the %s */ > str = va_arg(ap, const char *); > > @@ -3762,8 +3772,18 @@ void trace_check_vprintf(struct trace_iterator *iter, const char *fmt, > int ret; > > /* Try to safely read the string */ > - ret = strncpy_from_kernel_nofault(iter->fmt, str, > - iter->fmt_size); > + if (star) { > + if (len + 1 > iter->fmt_size) > + len = iter->fmt_size - 1; > + if (len < 0) > + len = 0; > + ret = copy_from_kernel_nofault(iter->fmt, str, len); > + iter->fmt[len] = 0; > + star = false; > + } else { > + ret = strncpy_from_kernel_nofault(iter->fmt, str, > + iter->fmt_size); > + } > if (ret < 0) > trace_seq_printf(&iter->seq, "(0x%px)", str); > else > @@ -3775,7 +3795,10 @@ void trace_check_vprintf(struct trace_iterator *iter, const char *fmt, > strncpy(iter->fmt, p + i, j + 1); > iter->fmt[j+1] = '\0'; > } > - trace_seq_printf(&iter->seq, iter->fmt, str); > + if (star) > + trace_seq_printf(&iter->seq, iter->fmt, len, str); > + else > + trace_seq_printf(&iter->seq, iter->fmt, str); > > p += i + j + 1; > } > -- > 2.29.2 > -- Chuck Lever
