linux-kernel - Re: [RFC PATCH v4 1/3] tracing/events: Fix wrong sample output by storing array length instead of size

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: <878ua6u0nf.fsf@linaro.org>
Date:	Thu, 23 Jul 2015 20:36:20 +0100
From:	Alex Bennée <alex.bennee@...aro.org>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	He Kuang <hekuang@...wei.com>, ast@...mgrid.com,
	masami.hiramatsu.pt@...achi.com, acme@...nel.org,
	a.p.zijlstra@...llo.nl, mingo@...hat.com, namhyung@...nel.org,
	jolsa@...nel.org, wangnan0@...wei.com, pi3orama@....com,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v4 1/3] tracing/events: Fix wrong sample output by storing array length instead of size


Steven Rostedt <rostedt@...dmis.org> writes:

> On Fri, 17 Jul 2015 10:32:15 -0400
> Steven Rostedt <rostedt@...dmis.org> wrote:
>
>
>> This change affects all callers of dymanic_array, not just bitmasks.
>> 
>> >  	__data_size += __item_length;
>> >  
>> >  #undef __string
>> 
>> BTW, if I revert commit ac01ce1410fc2 "tracing: Make
>> ftrace_print_array_seq compute buf_len" it works again.
>> 
>> I'm going to look into this some more, and maybe the answer is to go
>> back and just pass in buffer length here. I can't see what was broken
>> before that change.
>
> OK, the print_array() code is already being used by the thermal events
> and can't be changed. But we can't make the proposed change because
> that changes the user interface.
>
> What we can change is the sample code!
>
> -- Steve
>
> From 95de1e9721a2f9d05831a53d228e181a33001c55 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (Red Hat)" <rostedt@...dmis.org>
> Date: Fri, 17 Jul 2015 14:03:26 -0400
> Subject: [PATCH] tracing: Fix sample output of dynamic arrays
>
> He Kuang noticed that the trace event samples for arrays was broken:
>
> "The output result of trace_foo_bar event in traceevent samples is
>  wrong. This problem can be reproduced as following:
>
>   (Build kernel with SAMPLE_TRACE_EVENTS=m)
>
>   $ insmod trace-events-sample.ko
>
>   $ echo 1 > /sys/kernel/debug/tracing/events/sample-trace/foo_bar/enable
>
>   $ cat /sys/kernel/debug/tracing/trace
>
>   event-sample-980 [000] ....  43.649559: foo_bar: foo hello 21 0x15
>   BIT1|BIT3|0x10 {0x1,0x6f6f6e53,0xff007970,0xffffffff} Snoopy
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                  The array length is not right, should be {0x1}.
>   (ffffffff,ffffffff)
>
>   event-sample-980 [000] ....  44.653827: foo_bar: foo hello 22 0x16
>   BIT2|BIT3|0x10
>   {0x1,0x2,0x646e6147,0x666c61,0xffffffff,0xffffffff,0x750aeffe,0x7}
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                  The array length is not right, should be {0x1,0x2}.
>   Gandalf (ffffffff,ffffffff)"
>
> This was caused by an update to have __print_array()'s second parameter
> be the count of items in the array and not the size of the array.
>
> As there is already users of __print_array(), it can not change. But
> the sample code can and we can also improve on the documentation about
> __print_array() and __get_dynamic_array_len().
>
> Link: http://lkml.kernel.org/r/1436839171-31527-2-git-send-email-hekuang@huawei.com
>
> Fixes: ac01ce1410fc2 ("tracing: Make ftrace_print_array_seq compute buf_len")
> Reported-by: He Kuang <hekuang@...wei.com>
> Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
> ---
>  samples/trace_events/trace-events-sample.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h
> index 8965d1bb8811..125d6402f64f 100644
> --- a/samples/trace_events/trace-events-sample.h
> +++ b/samples/trace_events/trace-events-sample.h
> @@ -168,7 +168,10 @@
>   *
>   *      For __dynamic_array(int, foo, bar) use __get_dynamic_array(foo)
>   *            Use __get_dynamic_array_len(foo) to get the length of the array
> - *            saved.
> + *            saved. Note, __get_dynamic_array_len() returns the total allocated
> + *            length of the dynamic array; __print_array() expects the second
> + *            parameter to be the number of elements. To get that, the array length
> + *            needs to be divided by the element size.
>   *
>   *      For __string(foo, bar) use __get_str(foo)
>   *
> @@ -288,7 +291,7 @@ TRACE_EVENT(foo_bar,
>   *    This prints out the array that is defined by __array in a nice format.
>   */
>  		  __print_array(__get_dynamic_array(list),
> -				__get_dynamic_array_len(list),
> +				__get_dynamic_array_len(list) / sizeof(int),
>  				sizeof(int)),
>  		  __get_str(str), __get_bitmask(cpus))
>  );

Reviewed-by: Alex Bennée <alex.bennee@...aro.org>

-- 
Alex Bennée
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/