Re: [PATCH 3/3] tracing: don't set parser->cont if it has reached the end of input buffer

[Steven Rostedt]

On Tue,  9 Jan 2018 17:55:48 +0800
changbin.du@xxxxxxxxx wrote:

> From: Changbin Du <changbin.du@xxxxxxxxx>
> 
> We should not set parser->cont if it has reached the end of input buffer.
> And since some callers (like ftrace_graph_write()) treat it as an error
> condition if trace_parser_cont() returns true.

This will break existing use cases. In fact you are removing the entire
point of this code. It NEEDS to continue if it reached the end of the
input buffer.

I do things like:

 # cat file > set_ftrace_filter

where the file has a list of function names. It writes in blocks, and
it could very well have a function name split between two writes where
the write is at the end of the buffer but not finished writing the
function name.

> 
> For example, if userspace set 'set_ftrace_filter' by writing:
> write(3, "abcdefg", 7)

>From my point of view, the above isn't done writing the function name
yet and we SHOULD continue waiting for more input.

BIG NACK on this patch. Sorry.

I'm guessing you have some program that writes only the strlen() of
these strings. That's wrong, you need to write "strlen()+1". Write some
real white space between calls, it will work. Add a "write(fd, " ", 1)"
between calls if you need to. Please don't change the kernel to fix
some bad use case. Especially when your fix will break existing use
cases.

-- Steve

> 
> Then in the kernel function ftrace_regex_write(), ftrace_process_regex()
> will not be executed. The result is that the given filter will not be
> applied at all.
> 
> ftrace_regex_write() {
> 	...
> 	read = trace_get_user(parser, ubuf, cnt, ppos);
> 	if (read >= 0 && trace_parser_loaded(parser) &&
> 	    !trace_parser_cont(parser)) {
> 		ret = ftrace_process_regex(iter, parser->buffer,
> 					   parser->idx, enable);
> 		...
> 	}
> 	...
> }
> 
> Signed-off-by: Changbin Du <changbin.du@xxxxxxxxx>