[PATCH 0/3] [GIT PULL] tracing: some more fixes before my 4.2 pull request

[PATCH 0/3] [GIT PULL] tracing: some more fixes before my 4.2 pull request

[Steven Rostedt]

Linus,

This isn't my 4.2 pull request (yet). I found a few more bugs that
I would have sent to fix 4.1, but since 4.1 is already out, I'm
sending this before sending my 4.2 request (which is ready to go).

After fixing the previous filter issue reported by Vince Weaver,
I could not come up with a situation where the operand counter (cnt)
could go below zero, so I added a WARN_ON_ONCE(cnt < 0). Vince was
able to trigger that warn on with his fuzzer test, but didn't have
a filter input that caused it.

Later, Sasha Levin was able to trigger that same warning, and was
able to give me the filter string that triggered it. It was simply
a single operation ">".

I wrapped the filtering code in a userspace program such that I could
single step through the logic. With a single operator the operand
counter can legitimately go below zero, and should be reported to the
user as an error, but should not produce a kernel warning. The
WARN_ON_ONCE(cnt < 0) should be just a "if (cnt < 0) break;" and the
code following it will produce the error message for the user.

While debugging this, I found that there was another bug that let
the pointer to the filter string go beyond the filter string.
This too was fixed.

Finally, there was a typo in a stub function that only gets compiled
if trace events is disabled but tracing is enabled (I'm not even sure
that's possible).

Please pull the latest trace-fixes-4.1 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-fixes-4.1

Tag SHA1: 518521eb203ca82fce34c3e496c2d31af30e1ef4
Head SHA1: cc9e4bde03f2b4cfba52406c021364cbd2a4a0f3


Steven Rostedt (Red Hat) (3):
      tracing/filter: Do not WARN on operand count going below zero
      tracing/filter: Do not allow infix to exceed end of string
      tracing: Fix typo from "static inlin" to "static inline"

----
 kernel/trace/trace.h               |  2 +-
 kernel/trace/trace_events_filter.c | 10 +++++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

[PATCH 1/3] tracing/filter: Do not WARN on operand count going below zero

[Steven Rostedt]

[-- Attachment #1: 0001-tracing-filter-Do-not-WARN-on-operand-count-going-be.patch --]
[-- Type: text/plain, Size: 1542 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

When testing the fix for the trace filter, I could not come up with
a scenario where the operand count goes below zero, so I added a
WARN_ON_ONCE(cnt < 0) to the logic. But there is legitimate case
that it can happen (although the filter would be wrong).

 # echo '>' > /sys/kernel/debug/events/ext4/ext4_truncate_exit/filter

That is, a single operation without any operands will hit the path
where the WARN_ON_ONCE() can trigger. Although this is harmless,
and the filter is reported as a error. But instead of spitting out
a warning to the kernel dmesg, just fail nicely and report it via
the proper channels.

Link: http://lkml.kernel.org/r/558C6082.90608@oracle.com

Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Cc: stable@vger.kernel.org # 2.6.33+
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_events_filter.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 7f2e97ce71a7..2900d7723d97 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1385,7 +1385,9 @@ static int check_preds(struct filter_parse_state *ps)
 		if (elt->op != OP_NOT)
 			cnt--;
 		n_normal_preds++;
-		WARN_ON_ONCE(cnt < 0);
+		/* all ops should have operands */
+		if (cnt < 0)
+			break;
 	}
 
 	if (cnt != 1 || !n_normal_preds || n_logical_preds >= n_normal_preds) {
-- 
2.1.4

[PATCH 2/3] tracing/filter: Do not allow infix to exceed end of string

[Steven Rostedt]

[-- Attachment #1: 0002-tracing-filter-Do-not-allow-infix-to-exceed-end-of-s.patch --]
[-- Type: text/plain, Size: 1880 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

While debugging a WARN_ON() for filtering, I found that it is possible
for the filter string to be referenced after its end. With the filter:

 # echo '>' > /sys/kernel/debug/events/ext4/ext4_truncate_exit/filter

The filter_parse() function can call infix_get_op() which calls
infix_advance() that updates the infix filter pointers for the cnt
and tail without checking if the filter is already at the end, which
will put the cnt to zero and the tail beyond the end. The loop then calls
infix_next() that has

	ps->infix.cnt--;
	return ps->infix.string[ps->infix.tail++];

The cnt will now be below zero, and the tail that is returned is
already passed the end of the filter string. So far the allocation
of the filter string usually has some buffer that is zeroed out, but
if the filter string is of the exact size of the allocated buffer
there's no guarantee that the charater after the nul terminating
character will be zero.

Luckily, only root can write to the filter.

Cc: stable@vger.kernel.org # 2.6.33+
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_events_filter.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 2900d7723d97..52adf02d7619 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1056,6 +1056,9 @@ static void parse_init(struct filter_parse_state *ps,
 
 static char infix_next(struct filter_parse_state *ps)
 {
+	if (!ps->infix.cnt)
+		return 0;
+
 	ps->infix.cnt--;
 
 	return ps->infix.string[ps->infix.tail++];
@@ -1071,6 +1074,9 @@ static char infix_peek(struct filter_parse_state *ps)
 
 static void infix_advance(struct filter_parse_state *ps)
 {
+	if (!ps->infix.cnt)
+		return;
+
 	ps->infix.cnt--;
 	ps->infix.tail++;
 }
-- 
2.1.4

[PATCH 3/3] tracing: Fix typo from "static inlin" to "static inline"

[Steven Rostedt]

[-- Attachment #1: 0003-tracing-Fix-typo-from-static-inlin-to-static-inline.patch --]
[-- Type: text/plain, Size: 967 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

The trace.h header when called without CONFIG_EVENT_TRACING enabled
(seldom done), will not compile because of a typo in the protocol
of trace_event_enum_update().

Cc: stable@vger.kernel.org # 4.1+
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index d2612016de94..3d2ad5f83e94 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1312,7 +1312,7 @@ void trace_event_init(void);
 void trace_event_enum_update(struct trace_enum_map **map, int len);
 #else
 static inline void __init trace_event_init(void) { }
-static inlin void trace_event_enum_update(struct trace_enum_map **map, int len) { }
+static inline void trace_event_enum_update(struct trace_enum_map **map, int len) { }
 #endif
 
 extern struct trace_iterator *tracepoint_print_iter;
-- 
2.1.4