Messages in this thread | | | Date | Fri, 10 Feb 2017 14:50:06 +0900 | From | Masami Hiramatsu <> | Subject | Re: [RFC][PATCH] tracing: Have traceprobe_probes_write() not access userspace unnecessarily |
| |
On Thu, 9 Feb 2017 18:04:58 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
> > The code in traceprobe_probes_write() reads up to 4096 bytes from userpace > for each line. If userspace passes in several lines to execute, the code > will do a large read for each line, even though, it is highly likely that > the first read from userspace received all of the lines at one. > > I changed the logic to do a single read from userspace, and to only read > from userspace again if not all of the read from userspace made it in. > > I tested this by adding printk()s and writing files that would test -1, ==, > and +1 the buffer size, to make sure that there's no overflows and that if a > single line is written with +1 the buffer size, that it fails properly. >
Thanks Steve!
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
BTW, this can conflict with my previous patch.
https://lkml.org/lkml/2017/2/6/1048 https://lkml.org/lkml/2017/2/7/203
I'll update this. Ingo, Can I send these patch to Steve?
Thank you,
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > --- > kernel/trace/trace_probe.c | 48 ++++++++++++++++++++++++++++------------------ > 1 file changed, 29 insertions(+), 19 deletions(-) > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c > index 8c0553d..2a06f1f 100644 > --- a/kernel/trace/trace_probe.c > +++ b/kernel/trace/trace_probe.c > @@ -647,7 +647,7 @@ ssize_t traceprobe_probes_write(struct file *file, const char __user *buffer, > size_t count, loff_t *ppos, > int (*createfn)(int, char **)) > { > - char *kbuf, *tmp; > + char *kbuf, *buf, *tmp; > int ret = 0; > size_t done = 0; > size_t size; > @@ -667,27 +667,37 @@ ssize_t traceprobe_probes_write(struct file *file, const char __user *buffer, > goto out; > } > kbuf[size] = '\0'; > - tmp = strchr(kbuf, '\n'); > + buf = kbuf; > + do { > + tmp = strchr(buf, '\n'); > + if (tmp) { > + *tmp = '\0'; > + size = tmp - buf + 1; > + } else { > + size = strlen(buf); > + if (done + size < count) { > + if (buf != kbuf) > + break; > + pr_warn("Line length is too long: Should be less than %d\n", > + WRITE_BUFSIZE); > + ret = -EINVAL; > + goto out; > + } > + } > + done += size; > > - if (tmp) { > - *tmp = '\0'; > - size = tmp - kbuf + 1; > - } else if (done + size < count) { > - pr_warn("Line length is too long: Should be less than %d\n", > - WRITE_BUFSIZE); > - ret = -EINVAL; > - goto out; > - } > - done += size; > - /* Remove comments */ > - tmp = strchr(kbuf, '#'); > + /* Remove comments */ > + tmp = strchr(buf, '#'); > > - if (tmp) > - *tmp = '\0'; > + if (tmp) > + *tmp = '\0'; > > - ret = traceprobe_command(kbuf, createfn); > - if (ret) > - goto out; > + ret = traceprobe_command(buf, createfn); > + if (ret) > + goto out; > + buf += size; > + > + } while (done < count); > } > ret = done; > > -- > 2.9.3 >
-- Masami Hiramatsu <mhiramat@kernel.org>
| |