* [PATCH v2 0/3] tracing: Fix the parser when processing strings w/ or w/o terminated '\0'
@ 2018-01-15 11:41 changbin.du
2018-01-15 11:41 ` [PATCH v2 1/3] tracing: detect the string termination character when parsing user input string changbin.du
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: changbin.du @ 2018-01-15 11:41 UTC (permalink / raw)
To: rostedt
Cc: jolsa, peterz, mingo, alexander.shishkin, linux-kernel,
linux-perf-users, Changbin Du
From: Changbin Du <changbin.du@intel.com>
I found there are some problems in the tracing parser when I investiage the root
cause of issues mentioned in below patch.
https://patchwork.kernel.org/patch/10132953/
This serials can fix them.
Changbin Du (3):
tracing: detect the string termination character when parsing user
input string
tracing: clear parser->idx if parser gets nothing
tracing: make sure the parsed string always terminates with '\0'
kernel/trace/ftrace.c | 2 --
kernel/trace/trace.c | 14 +++++++-------
kernel/trace/trace_events.c | 2 --
3 files changed, 7 insertions(+), 11 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/3] tracing: detect the string termination character when parsing user input string
2018-01-15 11:41 [PATCH v2 0/3] tracing: Fix the parser when processing strings w/ or w/o terminated '\0' changbin.du
@ 2018-01-15 11:41 ` changbin.du
2018-01-15 23:20 ` Steven Rostedt
2018-01-15 11:41 ` [PATCH v2 2/3] tracing: clear parser->idx if parser gets nothing changbin.du
2018-01-15 11:41 ` [PATCH v2 3/3] tracing: make sure the parsed string always terminates with '\0' changbin.du
2 siblings, 1 reply; 6+ messages in thread
From: changbin.du @ 2018-01-15 11:41 UTC (permalink / raw)
To: rostedt
Cc: jolsa, peterz, mingo, alexander.shishkin, linux-kernel,
linux-perf-users, Changbin Du
From: Changbin Du <changbin.du@intel.com>
The usersapce can give a '\0' terminated C string in the input buffer.
Before this change, trace_get_user() will return a parsed string "\0" in
below case which is not expected (expects it skip all inputs) and cause the
caller failed.
open("/sys/kernel/debug/tracing//set_ftrace_pid", O_WRONLY|O_TRUNC) = 3
write(3, " \0", 2) = -1 EINVAL (Invalid argument)
while parse can handle spaces, so below works.
$ echo "" > set_ftrace_pid
$ echo " " > set_ftrace_pid
$ echo -n " " > set_ftrace_pid
This patch try to make the parser '\0' aware to fix such issue. When parser
sees a '\0' it stops further parsing. With this change, write(3, " \0", 2)
will work.
Signed-off-by: Changbin Du <changbin.du@intel.com>
---
v2: Stop parsing when '\0' found.
---
kernel/trace/trace.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2a8d8a2..144d08e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1237,7 +1237,7 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
}
/* only spaces were written */
- if (isspace(ch)) {
+ if (isspace(ch) || !ch) {
*ppos += read;
ret = read;
goto out;
@@ -1247,7 +1247,7 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
}
/* read the non-space input */
- while (cnt && !isspace(ch)) {
+ while (cnt && !isspace(ch) && ch) {
if (parser->idx < parser->size - 1)
parser->buffer[parser->idx++] = ch;
else {
@@ -1262,7 +1262,7 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
}
/* We either got finished input or we have to wait for another call. */
- if (isspace(ch)) {
+ if (isspace(ch) || !ch) {
parser->buffer[parser->idx] = 0;
parser->cont = false;
} else if (parser->idx < parser->size - 1) {
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] tracing: clear parser->idx if parser gets nothing
2018-01-15 11:41 [PATCH v2 0/3] tracing: Fix the parser when processing strings w/ or w/o terminated '\0' changbin.du
2018-01-15 11:41 ` [PATCH v2 1/3] tracing: detect the string termination character when parsing user input string changbin.du
@ 2018-01-15 11:41 ` changbin.du
2018-01-15 11:41 ` [PATCH v2 3/3] tracing: make sure the parsed string always terminates with '\0' changbin.du
2 siblings, 0 replies; 6+ messages in thread
From: changbin.du @ 2018-01-15 11:41 UTC (permalink / raw)
To: rostedt
Cc: jolsa, peterz, mingo, alexander.shishkin, linux-kernel,
linux-perf-users, Changbin Du
From: Changbin Du <changbin.du@intel.com>
If only spaces was got in that cycle, we should clear parser->idx to make
trace_parser_loaded() return false.
Signed-off-by: Changbin Du <changbin.du@intel.com>
---
kernel/trace/trace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 144d08e..b44926e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1236,14 +1236,14 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
cnt--;
}
+ parser->idx = 0;
+
/* only spaces were written */
if (isspace(ch) || !ch) {
*ppos += read;
ret = read;
goto out;
}
-
- parser->idx = 0;
}
/* read the non-space input */
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] tracing: make sure the parsed string always terminates with '\0'
2018-01-15 11:41 [PATCH v2 0/3] tracing: Fix the parser when processing strings w/ or w/o terminated '\0' changbin.du
2018-01-15 11:41 ` [PATCH v2 1/3] tracing: detect the string termination character when parsing user input string changbin.du
2018-01-15 11:41 ` [PATCH v2 2/3] tracing: clear parser->idx if parser gets nothing changbin.du
@ 2018-01-15 11:41 ` changbin.du
2 siblings, 0 replies; 6+ messages in thread
From: changbin.du @ 2018-01-15 11:41 UTC (permalink / raw)
To: rostedt
Cc: jolsa, peterz, mingo, alexander.shishkin, linux-kernel,
linux-perf-users, Changbin Du
From: Changbin Du <changbin.du@intel.com>
Always mark the parsed string with a terminated '\0' even parser expects
another input to be parsed. Thus the users needn't append '0' before
using parsed string if new input is not given.
Signed-off-by: Changbin Du <changbin.du@intel.com>
---
kernel/trace/ftrace.c | 2 --
kernel/trace/trace.c | 4 ++--
kernel/trace/trace_events.c | 2 --
3 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index ccdf366..3addb82 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5010,7 +5010,6 @@ int ftrace_regex_release(struct inode *inode, struct file *file)
parser = &iter->parser;
if (trace_parser_loaded(parser)) {
- parser->buffer[parser->idx] = 0;
ftrace_match_records(iter->hash, parser->buffer, parser->idx);
}
@@ -5324,7 +5323,6 @@ ftrace_graph_release(struct inode *inode, struct file *file)
parser = &fgd->parser;
if (trace_parser_loaded((parser))) {
- parser->buffer[parser->idx] = 0;
ret = ftrace_graph_set_hash(fgd->new_hash,
parser->buffer);
}
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index b44926e..8f7fea2 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -530,8 +530,6 @@ int trace_pid_write(struct trace_pid_list *filtered_pids,
ubuf += ret;
cnt -= ret;
- parser.buffer[parser.idx] = 0;
-
ret = -EINVAL;
if (kstrtoul(parser.buffer, 0, &val))
break;
@@ -1268,6 +1266,8 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
} else if (parser->idx < parser->size - 1) {
parser->cont = true;
parser->buffer[parser->idx++] = ch;
+ /* Make sure the parsed string always terminates with '\0'. */
+ parser->buffer[parser->idx] = 0;
} else {
ret = -EINVAL;
goto out;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index ec0f9aa..7f8027c 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -885,8 +885,6 @@ ftrace_event_write(struct file *file, const char __user *ubuf,
if (*parser.buffer == '!')
set = 0;
- parser.buffer[parser.idx] = 0;
-
ret = ftrace_set_clr_event(tr, parser.buffer + !set, set);
if (ret)
goto out_put;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] tracing: detect the string termination character when parsing user input string
2018-01-15 11:41 ` [PATCH v2 1/3] tracing: detect the string termination character when parsing user input string changbin.du
@ 2018-01-15 23:20 ` Steven Rostedt
2018-01-16 2:41 ` Du, Changbin
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2018-01-15 23:20 UTC (permalink / raw)
To: changbin.du
Cc: jolsa, peterz, mingo, alexander.shishkin, linux-kernel, linux-perf-users
On Mon, 15 Jan 2018 19:41:12 +0800
changbin.du@intel.com wrote:
> From: Changbin Du <changbin.du@intel.com>
>
> The usersapce can give a '\0' terminated C string in the input buffer.
> Before this change, trace_get_user() will return a parsed string "\0" in
> below case which is not expected (expects it skip all inputs) and cause the
> caller failed.
The above totally does not parse (no pun intended).
Are you trying to say:
"User space can pass in a C nul character '\0' along with its input.
The function trace_get_user() will try to process it as a normal
character, and that will fail to parse.
>
> open("/sys/kernel/debug/tracing//set_ftrace_pid", O_WRONLY|O_TRUNC) = 3
> write(3, " \0", 2) = -1 EINVAL (Invalid argument)
>
> while parse can handle spaces, so below works.
>
> $ echo "" > set_ftrace_pid
> $ echo " " > set_ftrace_pid
> $ echo -n " " > set_ftrace_pid
>
> This patch try to make the parser '\0' aware to fix such issue. When parser
> sees a '\0' it stops further parsing. With this change, write(3, " \0", 2)
> will work.
The above should be something like:
"Have the parser stop on '\0' and cease any further parsing. Only
process the characters up to the nul '\0' character and do not process
it."
-- Steve
>
> Signed-off-by: Changbin Du <changbin.du@intel.com>
>
> ---
> v2: Stop parsing when '\0' found.
> ---
> kernel/trace/trace.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 2a8d8a2..144d08e 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -1237,7 +1237,7 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
> }
>
> /* only spaces were written */
> - if (isspace(ch)) {
> + if (isspace(ch) || !ch) {
> *ppos += read;
> ret = read;
> goto out;
> @@ -1247,7 +1247,7 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
> }
>
> /* read the non-space input */
> - while (cnt && !isspace(ch)) {
> + while (cnt && !isspace(ch) && ch) {
> if (parser->idx < parser->size - 1)
> parser->buffer[parser->idx++] = ch;
> else {
> @@ -1262,7 +1262,7 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
> }
>
> /* We either got finished input or we have to wait for another call. */
> - if (isspace(ch)) {
> + if (isspace(ch) || !ch) {
> parser->buffer[parser->idx] = 0;
> parser->cont = false;
> } else if (parser->idx < parser->size - 1) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] tracing: detect the string termination character when parsing user input string
2018-01-15 23:20 ` Steven Rostedt
@ 2018-01-16 2:41 ` Du, Changbin
0 siblings, 0 replies; 6+ messages in thread
From: Du, Changbin @ 2018-01-16 2:41 UTC (permalink / raw)
To: Steven Rostedt
Cc: changbin.du, jolsa, peterz, mingo, alexander.shishkin,
linux-kernel, linux-perf-users
Hi Rostedt,
Thanks for your polish, let me update commit msg with your words.
On Mon, Jan 15, 2018 at 06:20:00PM -0500, Steven Rostedt wrote:
> On Mon, 15 Jan 2018 19:41:12 +0800
> changbin.du@intel.com wrote:
>
> > From: Changbin Du <changbin.du@intel.com>
> >
> > The usersapce can give a '\0' terminated C string in the input buffer.
> > Before this change, trace_get_user() will return a parsed string "\0" in
> > below case which is not expected (expects it skip all inputs) and cause the
> > caller failed.
>
> The above totally does not parse (no pun intended).
>
> Are you trying to say:
>
> "User space can pass in a C nul character '\0' along with its input.
> The function trace_get_user() will try to process it as a normal
> character, and that will fail to parse.
>
> >
> > open("/sys/kernel/debug/tracing//set_ftrace_pid", O_WRONLY|O_TRUNC) = 3
> > write(3, " \0", 2) = -1 EINVAL (Invalid argument)
> >
> > while parse can handle spaces, so below works.
> >
> > $ echo "" > set_ftrace_pid
> > $ echo " " > set_ftrace_pid
> > $ echo -n " " > set_ftrace_pid
> >
> > This patch try to make the parser '\0' aware to fix such issue. When parser
> > sees a '\0' it stops further parsing. With this change, write(3, " \0", 2)
> > will work.
>
> The above should be something like:
>
> "Have the parser stop on '\0' and cease any further parsing. Only
> process the characters up to the nul '\0' character and do not process
> it."
>
> -- Steve
>
>
> >
> > Signed-off-by: Changbin Du <changbin.du@intel.com>
> >
> > ---
> > v2: Stop parsing when '\0' found.
> > ---
> > kernel/trace/trace.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 2a8d8a2..144d08e 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -1237,7 +1237,7 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
> > }
> >
> > /* only spaces were written */
> > - if (isspace(ch)) {
> > + if (isspace(ch) || !ch) {
> > *ppos += read;
> > ret = read;
> > goto out;
> > @@ -1247,7 +1247,7 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
> > }
> >
> > /* read the non-space input */
> > - while (cnt && !isspace(ch)) {
> > + while (cnt && !isspace(ch) && ch) {
> > if (parser->idx < parser->size - 1)
> > parser->buffer[parser->idx++] = ch;
> > else {
> > @@ -1262,7 +1262,7 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
> > }
> >
> > /* We either got finished input or we have to wait for another call. */
> > - if (isspace(ch)) {
> > + if (isspace(ch) || !ch) {
> > parser->buffer[parser->idx] = 0;
> > parser->cont = false;
> > } else if (parser->idx < parser->size - 1) {
>
--
Thanks,
Changbin Du
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-01-16 2:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-15 11:41 [PATCH v2 0/3] tracing: Fix the parser when processing strings w/ or w/o terminated '\0' changbin.du
2018-01-15 11:41 ` [PATCH v2 1/3] tracing: detect the string termination character when parsing user input string changbin.du
2018-01-15 23:20 ` Steven Rostedt
2018-01-16 2:41 ` Du, Changbin
2018-01-15 11:41 ` [PATCH v2 2/3] tracing: clear parser->idx if parser gets nothing changbin.du
2018-01-15 11:41 ` [PATCH v2 3/3] tracing: make sure the parsed string always terminates with '\0' changbin.du
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.