Re: [PATCH] perf: Avoid -Warray-bounds warning for __rel_loc macro

[Masami Hiramatsu]

On Tue, 25 Jan 2022 14:00:37 -0800
Kees Cook <keescook@xxxxxxxxxxxx> wrote:

> As done for trace_events.h, also fix the __rel_loc macro in perf.h,
> which silences the -Warray-bounds warning:
> 
> In file included from ./include/linux/string.h:253,
>                  from ./include/linux/bitmap.h:11,
>                  from ./include/linux/cpumask.h:12,
>                  from ./include/linux/mm_types_task.h:14,
>                  from ./include/linux/mm_types.h:5,
>                  from ./include/linux/buildid.h:5,
>                  from ./include/linux/module.h:14,
>                  from samples/trace_events/trace-events-sample.c:2:
> In function '__fortify_strcpy',
>     inlined from 'perf_trace_foo_rel_loc' at samples/trace_events/./trace-events-sample.h:519:1:
> ./include/linux/fortify-string.h:47:33: warning: '__builtin_strcpy' offset 12 is out of the bounds [
> 0, 4] [-Warray-bounds]
>    47 | #define __underlying_strcpy     __builtin_strcpy
>       |                                 ^
> ./include/linux/fortify-string.h:445:24: note: in expansion of macro '__underlying_strcpy'
>   445 |                 return __underlying_strcpy(p, q);
>       |                        ^~~~~~~~~~~~~~~~~~~
> 
> Also make __data struct member a proper flexible array to avoid future
> problems.
> 

This looks good to me.

Reviewed-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>

BTW, same macro is in the include/trace/bpf_probe.h.
I'm not sure bpf using this macro, should we update it for
consistency?

> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> Reported-by: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
>  include/trace/perf.h         | 5 +++--
>  include/trace/trace_events.h | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/include/trace/perf.h b/include/trace/perf.h
> index b77d09c70a93..5800d13146c3 100644
> --- a/include/trace/perf.h
> +++ b/include/trace/perf.h
> @@ -26,8 +26,9 @@
>  
>  #undef __get_rel_dynamic_array
>  #define __get_rel_dynamic_array(field)	\
> -		((void *)(&__entry->__rel_loc_##field) +	\
> -		 sizeof(__entry->__rel_loc_##field) +		\
> +		((void *)__entry +					\
> +		 offsetof(typeof(*__entry), __rel_loc_##field) +	\
> +		 sizeof(__entry->__rel_loc_##field) +			\
>  		 (__entry->__rel_loc_##field & 0xffff))
>  
>  #undef __get_rel_dynamic_array_len
> diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
> index cefefed18e85..7c86cc541c7a 100644
> --- a/include/trace/trace_events.h
> +++ b/include/trace/trace_events.h
> @@ -134,7 +134,7 @@ TRACE_MAKE_SYSTEM_STR();
>  	struct trace_event_raw_##name {					\
>  		struct trace_entry	ent;				\
>  		tstruct							\
> -		char			__data[0];			\
> +		char			__data[];			\
>  	};								\
>  									\
>  	static struct trace_event_class event_class_##name;
> -- 
> 2.30.2
> 


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>