Re: linux-next: build failure after merge of the kspp tree

[Kees Cook <keescook@chromium.org>]

On Tue, Jan 25, 2022 at 02:50:06PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (powerpc
> allmodconfig) failed like this:
> 
> 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: error: '__builtin_strcpy' offset 12 is out of the bounds [0, 4] [-Werror=array-bounds]

-Warray-bounds thinks something is trying to get at offset 12 of an
object it thinks is only 4 bytes in size.

>    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);
>       |                        ^~~~~~~~~~~~~~~~~~~
> 
> Exposed by (probably) commit
> 
>   602670289b69 ("fortify: Detect struct member overflows in memcpy() at compile-time")
> 
> Introduced by commit
> 
>   b466b1332164 ("samples/trace_event: Add '__rel_loc' using sample event")
> 
> I have reverted that latter commit for today.

Digging through the macros, I end up reconstructing this:

	strcpy( (char *)((void *)(&__entry->__rel_loc_foo) +
				  sizeof(__entry->__rel_loc_foo) +
				  (__entry->__rel_loc_foo & 0xffff)),
		foo ? (const char *)(foo) : "(null)");

I couldn't figure out how __entry is being allocated, but it seemed
maybe related to this note:

/*
 * struct trace_event_data_offsets_<call> {
 *      u32                             <item1>;
 *      u32                             <item2>;
 *      [...]
 * };
 *
 * The __dynamic_array() macro will create each u32 <item>, this is
 * to keep the offset of each array from the beginning of the event.
 * The size of an array is also encoded, in the higher 16 bits of
 * <item>.
 */

So, I think -Warray-bounds is refusing to see the destination as
anything except a u32, but being accessed at 4 (sizeof(u32)) + 8
(address && 0xffff) (?)

But if this is true, I would imagine there would be plenty of other
warnings? I'm currently stumped.

Reading 55de2c0b5610 ("tracing: Add '__rel_loc' using trace event
macros") did not help me. ;)

-Kees

-- 
Kees Cook