From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751429AbdBJGVC (ORCPT ); Fri, 10 Feb 2017 01:21:02 -0500 Received: from LGEAMRELO13.lge.com ([156.147.23.53]:53677 "EHLO lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750979AbdBJGVB (ORCPT ); Fri, 10 Feb 2017 01:21:01 -0500 X-Original-SENDERIP: 156.147.1.127 X-Original-MAILFROM: namhyung@kernel.org X-Original-SENDERIP: 165.244.249.26 X-Original-MAILFROM: namhyung@kernel.org X-Original-SENDERIP: 10.177.227.17 X-Original-MAILFROM: namhyung@kernel.org Date: Fri, 10 Feb 2017 15:20:59 +0900 From: Namhyung Kim To: Steven Rostedt CC: LKML , Srikar Dronamraju , Masami Hiramatsu , Ingo Molnar , Andrew Morton Subject: Re: [RFC][PATCH] tracing: Have traceprobe_probes_write() not access userspace unnecessarily Message-ID: <20170210062059.GC14705@sejong> References: <20170209180458.5c829ab2@gandalf.local.home> MIME-Version: 1.0 In-Reply-To: <20170209180458.5c829ab2@gandalf.local.home> User-Agent: Mutt/1.7.2 (2016-11-26) X-MIMETrack: Itemize by SMTP Server on LGEKRMHUB03/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/02/10 15:20:58, Serialize by Router on LGEKRMHUB03/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/02/10 15:20:58, Serialize complete at 2017/02/10 15:20:58 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 09, 2017 at 06:04:58PM -0500, Steven Rostedt 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. > > Signed-off-by: Steven Rostedt (VMware) Acked-by: Namhyung Kim Thanks, Namhyung > --- > 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 >