linux-kernel - Re: [PATCH] tracing: Error if a trace event has an array for a _

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] [day] [month] [year] [list]

Message-Id: <20230311001914.9274a39d5291ff118cc85346@kernel.org>
Date:   Sat, 11 Mar 2023 00:19:14 +0900
From:   Masami Hiramatsu (Google) <mhiramat@...nel.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Douglas RAILLARD <douglas.raillard@....com>
Subject: Re: [PATCH] tracing: Error if a trace event has an array for a
 __field()

On Thu, 9 Mar 2023 22:13:02 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:

> From: "Steven Rostedt (Google)" <rostedt@...dmis.org>
> 
> A __field() in the TRACE_EVENT() macro is used to set up the fields of the
> trace event data. It is for single storage units (word, char, int,
> pointer, etc) and not for complex structures or arrays. Unfortunately,
> there's nothing preventing the build from accepting:
> 
>     __field(int, arr[5]);
> 
> from building. It will turn into a array value. This use to work fine, as
> the offset and size use to be determined by the macro using the field name,
> but things have changed and the offset and size are now determined by the
> type. So the above would only be size 4, and the next field will be
> located 4 bytes from it (instead of 20).
> 
> The proper way to declare static arrays is to use the __array() macro.
> 
> Instead of __field(int, arr[5]) it should be __array(int, arr, 5).
> 
> Add some macro tricks to the building of a trace event from the
> TRACE_EVENT() macro such that __field(int, arr[5]) will fail to build. A
> comment by the failure will explain why the build failed.
> 
> Link: https://lore.kernel.org/lkml/20230306122549.236561-1-douglas.raillard@arm.com/
> 

Looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>

Thank you!

> Reported-by: Douglas RAILLARD <douglas.raillard@....com>
> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> ---
>  include/trace/stages/stage5_get_offsets.h | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/include/trace/stages/stage5_get_offsets.h b/include/trace/stages/stage5_get_offsets.h
> index ac5c24d3beeb..e30a13be46ba 100644
> --- a/include/trace/stages/stage5_get_offsets.h
> +++ b/include/trace/stages/stage5_get_offsets.h
> @@ -9,17 +9,30 @@
>  #undef __entry
>  #define __entry entry
>  
> +/*
> + * Fields should never declare an array: i.e. __field(int, arr[5])
> + * If they do, it will cause issues in parsing and possibly corrupt the
> + * events. To prevent that from happening, test the sizeof() a fictitious
> + * type called "struct _test_no_array_##item" which will fail if "item"
> + * contains array elements (like "arr[5]").
> + *
> + * If you hit this, use __array(int, arr, 5) instead.
> + */
>  #undef __field
> -#define __field(type, item)
> +#define __field(type, item)					\
> +	{ (void)sizeof(struct _test_no_array_##item *); }
>  
>  #undef __field_ext
> -#define __field_ext(type, item, filter_type)
> +#define __field_ext(type, item, filter_type)			\
> +	{ (void)sizeof(struct _test_no_array_##item *); }
>  
>  #undef __field_struct
> -#define __field_struct(type, item)
> +#define __field_struct(type, item)				\
> +	{ (void)sizeof(struct _test_no_array_##item *); }
>  
>  #undef __field_struct_ext
> -#define __field_struct_ext(type, item, filter_type)
> +#define __field_struct_ext(type, item, filter_type)		\
> +	{ (void)sizeof(struct _test_no_array_##item *); }
>  
>  #undef __array
>  #define __array(type, item, len)
> -- 
> 2.39.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>