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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180109181241.6b3f5ffb@vmware.local.home>
Date:   Tue, 9 Jan 2018 18:12:41 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     changbin.du@...el.com
Cc:     jolsa@...hat.com, peterz@...radead.org, mingo@...hat.com,
        alexander.shishkin@...ux.intel.com, linux-kernel@...r.kernel.org,
        linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 3/3] tracing: don't set parser->cont if it has reached
 the end of input buffer

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

> From: Changbin Du <changbin.du@...el.com>
> 
> 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@...el.com>