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

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

[Stephen Rothwell]

Hi Kees,

After merging the kspp tree, today's linux-next build (x86_64
allmodconfig) failed like this:

In file included from include/linux/bitmap.h:8:0,
                 from include/linux/cpumask.h:11,
                 from arch/x86/include/asm/cpumask.h:4,
                 from arch/x86/include/asm/msr.h:10,
                 from arch/x86/include/asm/processor.h:20,
                 from arch/x86/include/asm/cpufeature.h:4,
                 from arch/x86/include/asm/thread_info.h:52,
                 from include/linux/thread_info.h:37,
                 from arch/x86/include/asm/preempt.h:6,
                 from include/linux/preempt.h:80,
                 from include/linux/spinlock.h:50,
                 from include/linux/mmzone.h:7,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:14,
                 from drivers/scsi/csiostor/csio_lnode.c:37:
In function 'memcpy',
    inlined from 'csio_append_attrib' at drivers/scsi/csiostor/csio_lnode.c:248:2,
    inlined from 'csio_ln_fdmi_dprt_cbfn' at drivers/scsi/csiostor/csio_lnode.c:471:2:
include/linux/string.h:309:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
    __read_overflow2();
    ^
In function 'memcpy',
    inlined from 'csio_append_attrib' at drivers/scsi/csiostor/csio_lnode.c:248:2,
    inlined from 'csio_ln_fdmi_rhba_cbfn' at drivers/scsi/csiostor/csio_lnode.c:337:2:
include/linux/string.h:309:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
    __read_overflow2();
    ^

Caused by commit

  b90d6eba50d7 ("include/linux/string.h: add the option of fortified string.h functions")

I have reverted that commit for today.

-- 
Cheers,
Stephen Rothwell

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

[Daniel Micay]

On Fri, 2017-06-16 at 11:30 +1000, Stephen Rothwell wrote:
> Hi Kees,
> 
> After merging the kspp tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> In file included from include/linux/bitmap.h:8:0,
>                  from include/linux/cpumask.h:11,
>                  from arch/x86/include/asm/cpumask.h:4,
>                  from arch/x86/include/asm/msr.h:10,
>                  from arch/x86/include/asm/processor.h:20,
>                  from arch/x86/include/asm/cpufeature.h:4,
>                  from arch/x86/include/asm/thread_info.h:52,
>                  from include/linux/thread_info.h:37,
>                  from arch/x86/include/asm/preempt.h:6,
>                  from include/linux/preempt.h:80,
>                  from include/linux/spinlock.h:50,
>                  from include/linux/mmzone.h:7,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:14,
>                  from drivers/scsi/csiostor/csio_lnode.c:37:
> In function 'memcpy',
>     inlined from 'csio_append_attrib' at
> drivers/scsi/csiostor/csio_lnode.c:248:2,
>     inlined from 'csio_ln_fdmi_dprt_cbfn' at
> drivers/scsi/csiostor/csio_lnode.c:471:2:
> include/linux/string.h:309:4: error: call to '__read_overflow2'
> declared with attribute error: detected read beyond size of object
> passed as 2nd parameter
>     __read_overflow2();
>     ^
> In function 'memcpy',
>     inlined from 'csio_append_attrib' at
> drivers/scsi/csiostor/csio_lnode.c:248:2,
>     inlined from 'csio_ln_fdmi_rhba_cbfn' at
> drivers/scsi/csiostor/csio_lnode.c:337:2:
> include/linux/string.h:309:4: error: call to '__read_overflow2'
> declared with attribute error: detected read beyond size of object
> passed as 2nd parameter
>     __read_overflow2();
>     ^
> 
> Caused by commit
> 
>   b90d6eba50d7 ("include/linux/string.h: add the option of fortified
> string.h functions")
> 
> I have reverted that commit for today.

That's this one: https://lkml.org/lkml/2017/5/9/613, which is in
https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/ in the
4.13/scsi-queue and for-next branches. I think that's why Kees didn't
include it but I get he needs to add that.

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

[Daniel Micay]

> https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/ in the
> 4.13/scsi-queue and for-next branches. I think that's why Kees didn't
> include it but I get he needs to add that.

s/get/guess/

Or is that repo supposed to get pulled into next?

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

[Kees Cook]

On Thu, Jun 15, 2017 at 7:51 PM, Daniel Micay <danielmicay@gmail.com> wrote:
> On Fri, 2017-06-16 at 11:30 +1000, Stephen Rothwell wrote:
>> Hi Kees,
>>
>> After merging the kspp tree, today's linux-next build (x86_64
>> allmodconfig) failed like this:
>>
>> In file included from include/linux/bitmap.h:8:0,
>>                  from include/linux/cpumask.h:11,
>>                  from arch/x86/include/asm/cpumask.h:4,
>>                  from arch/x86/include/asm/msr.h:10,
>>                  from arch/x86/include/asm/processor.h:20,
>>                  from arch/x86/include/asm/cpufeature.h:4,
>>                  from arch/x86/include/asm/thread_info.h:52,
>>                  from include/linux/thread_info.h:37,
>>                  from arch/x86/include/asm/preempt.h:6,
>>                  from include/linux/preempt.h:80,
>>                  from include/linux/spinlock.h:50,
>>                  from include/linux/mmzone.h:7,
>>                  from include/linux/gfp.h:5,
>>                  from include/linux/slab.h:14,
>>                  from drivers/scsi/csiostor/csio_lnode.c:37:
>> In function 'memcpy',
>>     inlined from 'csio_append_attrib' at
>> drivers/scsi/csiostor/csio_lnode.c:248:2,
>>     inlined from 'csio_ln_fdmi_dprt_cbfn' at
>> drivers/scsi/csiostor/csio_lnode.c:471:2:
>> include/linux/string.h:309:4: error: call to '__read_overflow2'
>> declared with attribute error: detected read beyond size of object
>> passed as 2nd parameter
>>     __read_overflow2();
>>     ^
>> In function 'memcpy',
>>     inlined from 'csio_append_attrib' at
>> drivers/scsi/csiostor/csio_lnode.c:248:2,
>>     inlined from 'csio_ln_fdmi_rhba_cbfn' at
>> drivers/scsi/csiostor/csio_lnode.c:337:2:
>> include/linux/string.h:309:4: error: call to '__read_overflow2'
>> declared with attribute error: detected read beyond size of object
>> passed as 2nd parameter
>>     __read_overflow2();
>>     ^
>>
>> Caused by commit
>>
>>   b90d6eba50d7 ("include/linux/string.h: add the option of fortified
>> string.h functions")
>>
>> I have reverted that commit for today.
>
> That's this one: https://lkml.org/lkml/2017/5/9/613, which is in
> https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/ in the
> 4.13/scsi-queue and for-next branches. I think that's why Kees didn't
> include it but I get he needs to add that.

I'm so confused -- isn't this in next? All the build tests I did were
against yesterday's -next which includes this from what I can see...

-Kees

-- 
Kees Cook
Pixel Security

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

[Stephen Rothwell]

Hi Kees,

On Thu, 15 Jun 2017 20:20:47 -0700 Kees Cook <keescook@google.com> wrote:
>
> I'm so confused -- isn't this in next? All the build tests I did were
> against yesterday's -next which includes this from what I can see...

It is in next, but gets merged after the kspp tree ... so, this is when
inter-tree dependencies are a pain - I can merge the kspp tree later,
but then you have to remember which trees Linus must merge before you
send your pull request.  That's why we like to have all trees be
effectively stand alone (as much as possible).

-- 
Cheers,
Stephen Rothwell

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

[Stephen Rothwell]

Hi Stephen,

On Fri, 16 Jun 2017 13:31:44 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Kees,
> 
> On Thu, 15 Jun 2017 20:20:47 -0700 Kees Cook <keescook@google.com> wrote:
> >
> > I'm so confused -- isn't this in next? All the build tests I did were
> > against yesterday's -next which includes this from what I can see...  
> 
> It is in next, but gets merged after the kspp tree ... so, this is when
> inter-tree dependencies are a pain - I can merge the kspp tree later,
> but then you have to remember which trees Linus must merge before you
> send your pull request.  That's why we like to have all trees be
> effectively stand alone (as much as possible).

OK, for now I have moved the merging of the kspp tree to after
everything else (except Andrew's quilt series).  This will
(unfortunately) hide some dependencies between trees.

-- 
Cheers,
Stephen Rothwell

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

[Kees Cook]

On Sun, Jun 18, 2017 at 5:23 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Stephen,
>
> On Fri, 16 Jun 2017 13:31:44 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> Hi Kees,
>>
>> On Thu, 15 Jun 2017 20:20:47 -0700 Kees Cook <keescook@google.com> wrote:
>> >
>> > I'm so confused -- isn't this in next? All the build tests I did were
>> > against yesterday's -next which includes this from what I can see...
>>
>> It is in next, but gets merged after the kspp tree ... so, this is when
>> inter-tree dependencies are a pain - I can merge the kspp tree later,
>> but then you have to remember which trees Linus must merge before you
>> send your pull request.  That's why we like to have all trees be
>> effectively stand alone (as much as possible).
>
> OK, for now I have moved the merging of the kspp tree to after
> everything else (except Andrew's quilt series).  This will
> (unfortunately) hide some dependencies between trees.

In the other thread Andrew asked that I just have it all go through
-mm, so I've removed it from KSPP and sent the series his way (with
you in Cc).

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

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

[Kees Cook]

On Tue, Mar 05, 2024 at 02:50:18PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> In file included from lib/string_kunit.c:8:
> lib/string_kunit.c: In function 'test_strspn':
> lib/string_kunit.c:176:25: error: format '%d' expects argument of type 'int', but argument 7 has type 'size_t' {aka 'long unsigned int'} [-Werror=format=]
>   176 |                         "i:%d", i);
>       |                         ^~~~~~  ~
>       |                                 |
>       |                                 size_t {aka long unsigned int}

I was really scratching my head on this one. I didn't see the warning on
my end because this is actually a result of the merge, namely KUnit
becoming correctly stricter about format strings:

806cb2270237 ("kunit: Annotate _MSG assertion variants with gnu printf specifiers")

I will fix the format string! :)

-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3397 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
allmodconfig) failed like this:

In file included from lib/string_kunit.c:8:
lib/string_kunit.c: In function 'test_strspn':
lib/string_kunit.c:176:25: error: format '%d' expects argument of type 'int', but argument 7 has type 'size_t' {aka 'long unsigned int'} [-Werror=format=]
  176 |                         "i:%d", i);
      |                         ^~~~~~  ~
      |                                 |
      |                                 size_t {aka long unsigned int}
include/kunit/test.h:597:37: note: in definition of macro '_KUNIT_FAILED'
  597 |                                     fmt,                                       \
      |                                     ^~~
include/kunit/test.h:722:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
  722 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:1253:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
 1253 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
lib/string_kunit.c:175:17: note: in expansion of macro 'KUNIT_ASSERT_EQ_MSG'
  175 |                 KUNIT_ASSERT_EQ_MSG(test, s->a, strspn(s->str, s->accept),
      |                 ^~~~~~~~~~~~~~~~~~~
lib/string_kunit.c:176:29: note: format string is defined here
  176 |                         "i:%d", i);
      |                            ~^
      |                             |
      |                             int
      |                            %ld
lib/string_kunit.c:178:25: error: format '%d' expects argument of type 'int', but argument 7 has type 'size_t' {aka 'long unsigned int'} [-Werror=format=]
  178 |                         "i:%d", i);
      |                         ^~~~~~  ~
      |                                 |
      |                                 size_t {aka long unsigned int}
include/kunit/test.h:597:37: note: in definition of macro '_KUNIT_FAILED'
  597 |                                     fmt,                                       \
      |                                     ^~~
include/kunit/test.h:722:9: note: in expansion of macro 'KUNIT_BASE_BINARY_ASSERTION'
  722 |         KUNIT_BASE_BINARY_ASSERTION(test,                                      \
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/kunit/test.h:1253:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
 1253 |         KUNIT_BINARY_INT_ASSERTION(test,                                       \
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
lib/string_kunit.c:177:17: note: in expansion of macro 'KUNIT_ASSERT_EQ_MSG'
  177 |                 KUNIT_ASSERT_EQ_MSG(test, s->r, strcspn(s->str, s->reject),
      |                 ^~~~~~~~~~~~~~~~~~~
lib/string_kunit.c:178:29: note: format string is defined here
  178 |                         "i:%d", i);
      |                            ~^
      |                             |
      |                             int
      |                            %ld
cc1: all warnings being treated as errors

Caused by commit

  578914ebade8 ("string: Convert selftest to KUnit")

I have used the kspp tree from next-20240304 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Thu, Sep 14, 2023 at 11:50:36AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
> 
> In file included from security/selinux/include/initial_sid_to_string.h:3,
>                  from scripts/selinux/genheaders/genheaders.c:19:
> include/uapi/linux/stddef.h:59:2: error: #endif without #if
>    59 | #endif /* _UAPI_LINUX_STDDEF_H */
>       |  ^~~~~
> 
> Caused by commit
> 
>   366c47121bf6 ("uapi: stddef.h: Fix header guard location")
> 
> I have used the kspp tree from next-20230913 for today.

Agh, thanks. That's what I get for last-minute tweak. All errors are
mine; Alexey is innocent. :)

I'll get it fixed.

-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 552 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

In file included from security/selinux/include/initial_sid_to_string.h:3,
                 from scripts/selinux/genheaders/genheaders.c:19:
include/uapi/linux/stddef.h:59:2: error: #endif without #if
   59 | #endif /* _UAPI_LINUX_STDDEF_H */
      |  ^~~~~

Caused by commit

  366c47121bf6 ("uapi: stddef.h: Fix header guard location")

I have used the kspp tree from next-20230913 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

Hi Kees,

On Wed, 4 Jan 2023 19:22:14 -0800 Kees Cook <keescook@chromium.org> wrote:
>
> Yes, that looks entirely correct. Are you sending that separately, or
> should I carry it in my tree?

Please add it to your tree, thanks.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Thu, Jan 05, 2023 at 01:25:35PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (x86_64
> 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 arch/x86/include/asm/paravirt.h:17,
>                  from arch/x86/include/asm/cpuid.h:62,
>                  from arch/x86/include/asm/processor.h:19,
>                  from arch/x86/include/asm/cpufeature.h:5,
>                  from arch/x86/include/asm/thread_info.h:53,
>                  from include/linux/thread_info.h:60,
>                  from arch/x86/include/asm/preempt.h:9,
>                  from include/linux/preempt.h:78,
>                  from include/linux/percpu.h:6,
>                  from include/linux/prandom.h:13,
>                  from include/linux/random.h:153,
>                  from include/linux/net.h:18,
>                  from net/rxrpc/output.c:10:
> In function 'fortify_memcpy_chk',
>     inlined from 'rxrpc_fill_out_ack' at net/rxrpc/output.c:158:2:
> include/linux/fortify-string.h:520:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>   520 |                         __write_overflow_field(p_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Exposed by commit
> 
>   f7cd05c76c70 ("fortify: Use __builtin_dynamic_object_size() when available")

Oh! I'm not sure how I missed that in test builds.

> I applied the following patch.
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Thu, 5 Jan 2023 12:56:39 +1100
> Subject: [PATCH] rxrpc: replace zero-lenth array with DECLARE_FLEX_ARRAY() helper
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  net/rxrpc/ar-internal.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
> index 18092526d3c8..c7186484fc5f 100644
> --- a/net/rxrpc/ar-internal.h
> +++ b/net/rxrpc/ar-internal.h
> @@ -783,7 +783,7 @@ struct rxrpc_txbuf {
>  			u8	data[RXRPC_JUMBO_DATALEN]; /* Data packet */
>  			struct {
>  				struct rxrpc_ackpacket ack;
> -				u8 acks[0];
> +				DECLARE_FLEX_ARRAY(u8, acks);
>  			};
>  		};
>  	} __aligned(64);

Yes, that looks entirely correct. Are you sending that separately, or
should I carry it in my tree?

Thanks!

-Kees

-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2340 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
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 arch/x86/include/asm/paravirt.h:17,
                 from arch/x86/include/asm/cpuid.h:62,
                 from arch/x86/include/asm/processor.h:19,
                 from arch/x86/include/asm/cpufeature.h:5,
                 from arch/x86/include/asm/thread_info.h:53,
                 from include/linux/thread_info.h:60,
                 from arch/x86/include/asm/preempt.h:9,
                 from include/linux/preempt.h:78,
                 from include/linux/percpu.h:6,
                 from include/linux/prandom.h:13,
                 from include/linux/random.h:153,
                 from include/linux/net.h:18,
                 from net/rxrpc/output.c:10:
In function 'fortify_memcpy_chk',
    inlined from 'rxrpc_fill_out_ack' at net/rxrpc/output.c:158:2:
include/linux/fortify-string.h:520:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
  520 |                         __write_overflow_field(p_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Exposed by commit

  f7cd05c76c70 ("fortify: Use __builtin_dynamic_object_size() when available")

I applied the following patch.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 5 Jan 2023 12:56:39 +1100
Subject: [PATCH] rxrpc: replace zero-lenth array with DECLARE_FLEX_ARRAY() helper

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 net/rxrpc/ar-internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 18092526d3c8..c7186484fc5f 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -783,7 +783,7 @@ struct rxrpc_txbuf {
 			u8	data[RXRPC_JUMBO_DATALEN]; /* Data packet */
 			struct {
 				struct rxrpc_ackpacket ack;
-				u8 acks[0];
+				DECLARE_FLEX_ARRAY(u8, acks);
 			};
 		};
 	} __aligned(64);
-- 
2.35.1

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Thu, Dec 01, 2022 at 02:07:58PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> kernel/panic.c: In function 'check_panic_on_warn':
> kernel/panic.c:238:25: error: format '%s' expects argument of type 'char *', but argument 2 has type 'unsigned int' [-Werror=format=]
>   238 |                 panic("%s: system warned too often (kernel.warn_limit is %d)",
>       |                        ~^

Ugh. Thank you. It looks like my unfixed tree got pushed; apologies.
This has been corrected now.

-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
allmodconfig) failed like this:

kernel/panic.c: In function 'check_panic_on_warn':
kernel/panic.c:238:25: error: format '%s' expects argument of type 'char *', but argument 2 has type 'unsigned int' [-Werror=format=]
  238 |                 panic("%s: system warned too often (kernel.warn_limit is %d)",
      |                        ~^
      |                         |
      |                         char *
      |                        %d
  239 |                       warn_limit);
      |                       ~~~~~~~~~~
      |                       |
      |                       unsigned int
kernel/panic.c:238:75: error: format '%d' expects a matching 'int' argument [-Werror=format=]
  238 |                 panic("%s: system warned too often (kernel.warn_limit is %d)",
      |                                                                          ~^
      |                                                                           |
      |                                                                           int

Caused by commit

  480e0e1d4817 ("panic: Introduce warn_limit")

I have used the kspp tree from next-20221130 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Matthias Kaehlcke]

Hi Stephen,

Thanks for letting me know and for the fix in -next.

I'll send out a fix (which will probably be the same as yours).

Thanks

Matthias

On Thu, Jul 28, 2022 at 08:55:43PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
> 
> drivers/md/dm-verity-loadpin.c: In function 'dm_verity_loadpin_is_bdev_trusted':
> drivers/md/dm-verity-loadpin.c:61:13: error: implicit declaration of function 'dm_table_get_num_targets'; did you mean 'dm_table_resume_targets'? [-Werror=implicit-function-declaration]
>    61 |         if (dm_table_get_num_targets(table) != 1)
>       |             ^~~~~~~~~~~~~~~~~~~~~~~~
>       |             dm_table_resume_targets
> drivers/md/dm-verity-loadpin.c:64:14: error: implicit declaration of function 'dm_table_get_target'; did you mean 'dm_table_add_target'? [-Werror=implicit-function-declaration]
>    64 |         ti = dm_table_get_target(table, 0);
>       |              ^~~~~~~~~~~~~~~~~~~
>       |              dm_table_add_target
> drivers/md/dm-verity-loadpin.c:64:12: error: assignment to 'struct dm_target *' from 'int' makes pointer from integer without a cast [-Werror=int-conversion]
>    64 |         ti = dm_table_get_target(table, 0);
>       |            ^
> 
> Caused by commit
> 
>   b6c1c5745ccc ("dm: Add verity helpers for LoadPin")
> 
> interacting with commits
> 
>   2aec377a2925 ("dm table: remove dm_table_get_num_targets() wrapper")
>   564b5c5476cd ("dm table: audit all dm_table_get_target() callers")
> 
> from the device-mapper tree.
> 
> I have applied the following merge fix patch for today.
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Thu, 28 Jul 2022 20:28:41 +1000
> Subject: [PATCH] fix up for "dm: Add verity helpers for LoadPin"
> 
> interacting with
> 
>   2aec377a2925 ("dm table: remove dm_table_get_num_targets() wrapper")
>   564b5c5476cd ("dm table: audit all dm_table_get_target() callers")
> 
> from the device-mapper tree.
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  drivers/md/dm-verity-loadpin.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-verity-loadpin.c b/drivers/md/dm-verity-loadpin.c
> index 10c18bc1652c..387ec43aef72 100644
> --- a/drivers/md/dm-verity-loadpin.c
> +++ b/drivers/md/dm-verity-loadpin.c
> @@ -5,6 +5,7 @@
>  #include <linux/dm-verity-loadpin.h>
>  
>  #include "dm.h"
> +#include "dm-core.h"
>  #include "dm-verity.h"
>  
>  #define DM_MSG_PREFIX	"verity-loadpin"
> @@ -58,7 +59,7 @@ bool dm_verity_loadpin_is_bdev_trusted(struct block_device *bdev)
>  
>  	table = dm_get_live_table(md, &srcu_idx);
>  
> -	if (dm_table_get_num_targets(table) != 1)
> +	if (table->num_targets != 1)
>  		goto out;
>  
>  	ti = dm_table_get_target(table, 0);
> -- 
> 2.35.1
> 
> -- 
> Cheers,
> Stephen Rothwell

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2603 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (powerpc
allyesconfig) failed like this:

drivers/md/dm-verity-loadpin.c: In function 'dm_verity_loadpin_is_bdev_trusted':
drivers/md/dm-verity-loadpin.c:61:13: error: implicit declaration of function 'dm_table_get_num_targets'; did you mean 'dm_table_resume_targets'? [-Werror=implicit-function-declaration]
   61 |         if (dm_table_get_num_targets(table) != 1)
      |             ^~~~~~~~~~~~~~~~~~~~~~~~
      |             dm_table_resume_targets
drivers/md/dm-verity-loadpin.c:64:14: error: implicit declaration of function 'dm_table_get_target'; did you mean 'dm_table_add_target'? [-Werror=implicit-function-declaration]
   64 |         ti = dm_table_get_target(table, 0);
      |              ^~~~~~~~~~~~~~~~~~~
      |              dm_table_add_target
drivers/md/dm-verity-loadpin.c:64:12: error: assignment to 'struct dm_target *' from 'int' makes pointer from integer without a cast [-Werror=int-conversion]
   64 |         ti = dm_table_get_target(table, 0);
      |            ^

Caused by commit

  b6c1c5745ccc ("dm: Add verity helpers for LoadPin")

interacting with commits

  2aec377a2925 ("dm table: remove dm_table_get_num_targets() wrapper")
  564b5c5476cd ("dm table: audit all dm_table_get_target() callers")

from the device-mapper tree.

I have applied the following merge fix patch for today.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 28 Jul 2022 20:28:41 +1000
Subject: [PATCH] fix up for "dm: Add verity helpers for LoadPin"

interacting with

  2aec377a2925 ("dm table: remove dm_table_get_num_targets() wrapper")
  564b5c5476cd ("dm table: audit all dm_table_get_target() callers")

from the device-mapper tree.

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/md/dm-verity-loadpin.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-verity-loadpin.c b/drivers/md/dm-verity-loadpin.c
index 10c18bc1652c..387ec43aef72 100644
--- a/drivers/md/dm-verity-loadpin.c
+++ b/drivers/md/dm-verity-loadpin.c
@@ -5,6 +5,7 @@
 #include <linux/dm-verity-loadpin.h>
 
 #include "dm.h"
+#include "dm-core.h"
 #include "dm-verity.h"
 
 #define DM_MSG_PREFIX	"verity-loadpin"
@@ -58,7 +59,7 @@ bool dm_verity_loadpin_is_bdev_trusted(struct block_device *bdev)
 
 	table = dm_get_live_table(md, &srcu_idx);
 
-	if (dm_table_get_num_targets(table) != 1)
+	if (table->num_targets != 1)
 		goto out;
 
 	ti = dm_table_get_target(table, 0);
-- 
2.35.1

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Thu, May 05, 2022 at 05:40:25PM +1000, Stephen Rothwell wrote:
> After merging the kspp tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> [...]
> 
> Caused by commit
> 
>   f4cfacd92972 ("lkdtm/stackleak: rework boundary management")
> 
> or maybe commit
> 
>   cbe7edb47d3c ("stackleak: rework stack low bound handling")
> 
> CONFIG_GCC_PLUGIN_STACKLEAK is not set for this build.
> 
> I have used the kspp tree from next-20220504 for today.

Thanks! Sorry for the glitch; Mark's fix should be visible now.

-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/misc/lkdtm/stackleak.c: In function 'check_stackleak_irqoff':
drivers/misc/lkdtm/stackleak.c:30:46: error: implicit declaration of function 'stackleak_task_low_bound' [-Werror=implicit-function-declaration]
   30 |         const unsigned long task_stack_low = stackleak_task_low_bound(current);
      |                                              ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/misc/lkdtm/stackleak.c:31:47: error: implicit declaration of function 'stackleak_task_high_bound'; did you mean 'stackleak_task_init'? [-Werror=implicit-function-declaration]
   31 |         const unsigned long task_stack_high = stackleak_task_high_bound(current);
      |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~
      |                                               stackleak_task_init
drivers/misc/lkdtm/stackleak.c:33:50: error: 'struct task_struct' has no member named 'lowest_stack'; did you mean 'ret_stack'?
   33 |         const unsigned long lowest_sp = current->lowest_stack;
      |                                                  ^~~~~~~~~~~~
      |                                                  ret_stack
drivers/misc/lkdtm/stackleak.c:74:23: error: implicit declaration of function 'stackleak_find_top_of_poison' [-Werror=implicit-function-declaration]
   74 |         poison_high = stackleak_find_top_of_poison(task_stack_low, untracked_high);
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Caused by commit

  f4cfacd92972 ("lkdtm/stackleak: rework boundary management")

or maybe commit

  cbe7edb47d3c ("stackleak: rework stack low bound handling")

CONFIG_GCC_PLUGIN_STACKLEAK is not set for this build.

I have used the kspp tree from next-20220504 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Marc Zyngier]

On 2022-03-17 01:01, Linus Walleij wrote:
> On Wed, Mar 16, 2022 at 8:32 AM Stephen Rothwell <sfr@canb.auug.org.au> 
> wrote:
> 
>> After merging the kspp tree, today's linux-next build (x86_64
>> allmodconfig) failed like this:
> (...)
>> I have applied the following hack patch for today.
>> 
>> From: Stephen Rothwell <sfr@canb.auug.org.au>
>> Date: Wed, 16 Mar 2022 18:12:14 +1100
>> Subject: [PATCH] fixup for "pinctrl: nuvoton: add NPCM7xx pinctrl and 
>> GPIO driver"
> 
> Patch looks good but didn't apply to he pinctrl tree when I
> tried to simply apply it there.
> 
> Since it is touching IRQ handlers, I suspect this could maybe
> apply to Marc Z:s or Bartosz trees? Or can you see which one
> needs this?

Yes, I've now picked this up.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

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

[Linus Walleij]

On Wed, Mar 16, 2022 at 8:32 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> After merging the kspp tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
(...)
> I have applied the following hack patch for today.
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Wed, 16 Mar 2022 18:12:14 +1100
> Subject: [PATCH] fixup for "pinctrl: nuvoton: add NPCM7xx pinctrl and GPIO driver"

Patch looks good but didn't apply to he pinctrl tree when I
tried to simply apply it there.

Since it is touching IRQ handlers, I suspect this could maybe
apply to Marc Z:s or Bartosz trees? Or can you see which one
needs this?

Yours,
Linus Walleij

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2882 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
allmodconfig) failed like this:

In file included from include/linux/bitmap.h:9,
                 from include/linux/cpumask.h:12,
                 from arch/x86/include/asm/paravirt.h:17,
                 from arch/x86/include/asm/irqflags.h:63,
                 from include/linux/irqflags.h:16,
                 from include/linux/rcupdate.h:26,
                 from include/linux/rculist.h:11,
                 from include/linux/pid.h:5,
                 from include/linux/sched.h:14,
                 from include/linux/ratelimit.h:6,
                 from include/linux/dev_printk.h:16,
                 from include/linux/device.h:15,
                 from drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c:5:
drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c: In function 'npcmgpio_irq_handler':
include/linux/find.h:40:23: error: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Werror=array-bounds]
   40 |                 val = *addr & GENMASK(size - 1, offset);
      |                       ^~~~~
drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c:219:13: note: while referencing 'sts'
  219 |         u32 sts, en, bit;
      |             ^~~

Caused by commit

  3b588e43ee5c ("pinctrl: nuvoton: add NPCM7xx pinctrl and GPIO driver")

probably exposed by commit

  a1d1e0e3d80a ("pinctrl: nuvoton: Add driver for WPCM450")

from the pinctrl tree and enabling -Werror=array-bounds.

I have applied the following hack patch for today.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 16 Mar 2022 18:12:14 +1100
Subject: [PATCH] fixup for "pinctrl: nuvoton: add NPCM7xx pinctrl and GPIO driver"

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c b/drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c
index 41136f63014a..fddcb7d6bdf4 100644
--- a/drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c
+++ b/drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c
@@ -216,7 +216,8 @@ static void npcmgpio_irq_handler(struct irq_desc *desc)
 	struct gpio_chip *gc;
 	struct irq_chip *chip;
 	struct npcm7xx_gpio *bank;
-	u32 sts, en, bit;
+	unsigned long sts, bit;
+	u32 en;
 
 	gc = irq_desc_get_handler_data(desc);
 	bank = gpiochip_get_data(gc);
@@ -225,7 +226,7 @@ static void npcmgpio_irq_handler(struct irq_desc *desc)
 	chained_irq_enter(chip, desc);
 	sts = ioread32(bank->base + NPCM7XX_GP_N_EVST);
 	en  = ioread32(bank->base + NPCM7XX_GP_N_EVEN);
-	dev_dbg(bank->gc.parent, "==> got irq sts %.8x %.8x\n", sts,
+	dev_dbg(bank->gc.parent, "==> got irq sts %.8lx %.8x\n", sts,
 		en);
 
 	sts &= en;
-- 
2.34.1

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Wed, Mar 09, 2022 at 05:58:56PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 3/9/22 17:52, Kees Cook wrote:
> > On Wed, Mar 09, 2022 at 09:10:29PM +1100, Stephen Rothwell wrote:
> >> Hi all,
> >>
> >> After merging the kspp tree, today's linux-next build (x86_64
> >> allmodconfig) failed like this:
> >>
> >> In file included from include/linux/bitops.h:33,
> >>                  from include/linux/kernel.h:22,
> >>                  from drivers/platform/x86/thinkpad_acpi.c:37:
> >> drivers/platform/x86/thinkpad_acpi.c: In function 'tpacpi_dytc_profile_init':
> >> arch/x86/include/asm/bitops.h:207:22: error: array subscript 'long unsigned int[0]' is partly outside array bounds of 'int[1]' [-Werror=array-bounds]
> >>   207 |                 (addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
> >>       |                 ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> drivers/platform/x86/thinkpad_acpi.c:10385:18: note: while referencing 'output'
> >> 10385 |         int err, output;
> >>       |                  ^~~~~~
> >>
> >> Actually the test_bit() in line 10409.
> >>
> >> Caused by commit
> >>
> >>   e1c21608e3cf ("platform/x86: thinkpad_acpi: Add PSC mode support")
> >>
> >> from the drivers-x86 tree interacting with the enablement of
> >> -Werror=array-bounds.
> >>
> >> I have reverted that commit for today.
> 
> Thank you.
> 
> > Looking at https://lore.kernel.org/all/20220225182505.7234-1-markpearson@lenovo.com/
> > 
> > The problem is with the cast of "int output" to "unsigned long". Past
> > fixes have either made the variable unsigned long, or adding a new
> > variable and removal of casts. e.g. this pseudo-patch:
> > 
> > +	unsigned long bits;
> > 
> > 	...
> > +	bits = output;
> > -	if (test_bit(DYTC_FC_MMC, (void *)&output)) { /* MMC MODE */
> > +	if (test_bit(DYTC_FC_MMC, &bits)) { /* MMC MODE */
> > 
> > ...
> > 
> > -	} else if (test_bit(DYTC_FC_PSC, (void *)&output)) { /*PSC MODE */
> > +	} else if (test_bit(DYTC_FC_PSC, &bits)) { /*PSC MODE */
> 
> Right I was just preparing a patch for this, I've gone with:
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 7016c7fc3440..c476a78599d6 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -10399,7 +10399,7 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
>  	if (err)
>  		return err;
>  
> -	if (test_bit(DYTC_FC_MMC, (void *)&output)) { /* MMC MODE */
> +	if (output & BIT(DYTC_FC_MMC)) { /* MMC MODE */
>  		dytc_profile_available = DYTC_FUNCMODE_MMC;
>  
>  		/*
> @@ -10412,7 +10412,7 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
>  			if (!err && ((output & DYTC_ERR_MASK) == DYTC_ERR_SUCCESS))
>  				dytc_mmc_get_available = true;
>  		}
> -	} else if (test_bit(DYTC_FC_PSC, (void *)&output)) { /*PSC MODE */
> +	} else if (output & BIT(DYTC_FC_PSC)) { /* PSC MODE */
>  		dytc_profile_available = DYTC_FUNCMODE_PSC;
>  	} else {
>  		dbg_printk(TPACPI_DBG_INIT, "No DYTC support available\n");
> 
> I'll push this to the drivers-x86 tree for-next branch.

Ah yeah. Heh, that's much simpler. :)

-- 
Kees Cook

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

[Hans de Goede]

Hi,

On 3/9/22 17:52, Kees Cook wrote:
> On Wed, Mar 09, 2022 at 09:10:29PM +1100, Stephen Rothwell wrote:
>> Hi all,
>>
>> After merging the kspp tree, today's linux-next build (x86_64
>> allmodconfig) failed like this:
>>
>> In file included from include/linux/bitops.h:33,
>>                  from include/linux/kernel.h:22,
>>                  from drivers/platform/x86/thinkpad_acpi.c:37:
>> drivers/platform/x86/thinkpad_acpi.c: In function 'tpacpi_dytc_profile_init':
>> arch/x86/include/asm/bitops.h:207:22: error: array subscript 'long unsigned int[0]' is partly outside array bounds of 'int[1]' [-Werror=array-bounds]
>>   207 |                 (addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
>>       |                 ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/platform/x86/thinkpad_acpi.c:10385:18: note: while referencing 'output'
>> 10385 |         int err, output;
>>       |                  ^~~~~~
>>
>> Actually the test_bit() in line 10409.
>>
>> Caused by commit
>>
>>   e1c21608e3cf ("platform/x86: thinkpad_acpi: Add PSC mode support")
>>
>> from the drivers-x86 tree interacting with the enablement of
>> -Werror=array-bounds.
>>
>> I have reverted that commit for today.

Thank you.

> Looking at https://lore.kernel.org/all/20220225182505.7234-1-markpearson@lenovo.com/
> 
> The problem is with the cast of "int output" to "unsigned long". Past
> fixes have either made the variable unsigned long, or adding a new
> variable and removal of casts. e.g. this pseudo-patch:
> 
> +	unsigned long bits;
> 
> 	...
> +	bits = output;
> -	if (test_bit(DYTC_FC_MMC, (void *)&output)) { /* MMC MODE */
> +	if (test_bit(DYTC_FC_MMC, &bits)) { /* MMC MODE */
> 
> ...
> 
> -	} else if (test_bit(DYTC_FC_PSC, (void *)&output)) { /*PSC MODE */
> +	} else if (test_bit(DYTC_FC_PSC, &bits)) { /*PSC MODE */

Right I was just preparing a patch for this, I've gone with:

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 7016c7fc3440..c476a78599d6 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -10399,7 +10399,7 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
 	if (err)
 		return err;
 
-	if (test_bit(DYTC_FC_MMC, (void *)&output)) { /* MMC MODE */
+	if (output & BIT(DYTC_FC_MMC)) { /* MMC MODE */
 		dytc_profile_available = DYTC_FUNCMODE_MMC;
 
 		/*
@@ -10412,7 +10412,7 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
 			if (!err && ((output & DYTC_ERR_MASK) == DYTC_ERR_SUCCESS))
 				dytc_mmc_get_available = true;
 		}
-	} else if (test_bit(DYTC_FC_PSC, (void *)&output)) { /*PSC MODE */
+	} else if (output & BIT(DYTC_FC_PSC)) { /* PSC MODE */
 		dytc_profile_available = DYTC_FUNCMODE_PSC;
 	} else {
 		dbg_printk(TPACPI_DBG_INIT, "No DYTC support available\n");

I'll push this to the drivers-x86 tree for-next branch.

Regards,

Hans

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

[Kees Cook]

On Wed, Mar 09, 2022 at 09:10:29PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> In file included from include/linux/bitops.h:33,
>                  from include/linux/kernel.h:22,
>                  from drivers/platform/x86/thinkpad_acpi.c:37:
> drivers/platform/x86/thinkpad_acpi.c: In function 'tpacpi_dytc_profile_init':
> arch/x86/include/asm/bitops.h:207:22: error: array subscript 'long unsigned int[0]' is partly outside array bounds of 'int[1]' [-Werror=array-bounds]
>   207 |                 (addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
>       |                 ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/platform/x86/thinkpad_acpi.c:10385:18: note: while referencing 'output'
> 10385 |         int err, output;
>       |                  ^~~~~~
> 
> Actually the test_bit() in line 10409.
> 
> Caused by commit
> 
>   e1c21608e3cf ("platform/x86: thinkpad_acpi: Add PSC mode support")
> 
> from the drivers-x86 tree interacting with the enablement of
> -Werror=array-bounds.
> 
> I have reverted that commit for today.

Looking at https://lore.kernel.org/all/20220225182505.7234-1-markpearson@lenovo.com/

The problem is with the cast of "int output" to "unsigned long". Past
fixes have either made the variable unsigned long, or adding a new
variable and removal of casts. e.g. this pseudo-patch:

+	unsigned long bits;

	...
+	bits = output;
-	if (test_bit(DYTC_FC_MMC, (void *)&output)) { /* MMC MODE */
+	if (test_bit(DYTC_FC_MMC, &bits)) { /* MMC MODE */

...

-	} else if (test_bit(DYTC_FC_PSC, (void *)&output)) { /*PSC MODE */
+	} else if (test_bit(DYTC_FC_PSC, &bits)) { /*PSC MODE */



-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
allmodconfig) failed like this:

In file included from include/linux/bitops.h:33,
                 from include/linux/kernel.h:22,
                 from drivers/platform/x86/thinkpad_acpi.c:37:
drivers/platform/x86/thinkpad_acpi.c: In function 'tpacpi_dytc_profile_init':
arch/x86/include/asm/bitops.h:207:22: error: array subscript 'long unsigned int[0]' is partly outside array bounds of 'int[1]' [-Werror=array-bounds]
  207 |                 (addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
      |                 ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/platform/x86/thinkpad_acpi.c:10385:18: note: while referencing 'output'
10385 |         int err, output;
      |                  ^~~~~~

Actually the test_bit() in line 10409.

Caused by commit

  e1c21608e3cf ("platform/x86: thinkpad_acpi: Add PSC mode support")

from the drivers-x86 tree interacting with the enablement of
-Werror=array-bounds.

I have reverted that commit for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

Hi all,

ok, today I used the current kspp tree and added three patches as merge fixups:

https://lore.kernel.org/all/20220228081421.1504213-1-hsinyi@chromium.org/
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d4da1f27396fb1dde079447a3612f4f512caed07
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a2151490cc6c57b368d7974ffd447a8b36ade639

(the second one required a bit of fuzzing)

I will remove them as they appear in the drm trees (hopefully).

Kees, you just need to remember that you have a dependency on the DRM
patches being in Linus' tree before you send your pull request.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 4182 bytes --]

Hi Kees,

On Mon, 28 Feb 2022 15:02:48 -0800 Kees Cook <keescook@chromium.org> wrote:
>
> On Tue, Mar 01, 2022 at 09:27:30AM +1100, Stephen Rothwell wrote:
> > 
> > After merging the kspp tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> > 
> > drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_pcon_dsc_bpp_incr':
> > drivers/gpu/drm/drm_dp_helper.c:3130:28: error: array subscript 12 is outside array bounds of 'const u8[12]' {aka 'const unsigned char[12]'} [-Werror=array-bounds]
> >  3130 |         buf = pcon_dsc_dpcd[DP_PCON_DSC_BPP_INCR - DP_PCON_DSC_ENCODER];
> >       |               ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/gpu/drm/drm_dp_helper.c:3126:39: note: while referencing 'pcon_dsc_dpcd'
> >  3126 | int drm_dp_pcon_dsc_bpp_incr(const u8 pcon_dsc_dpcd[DP_PCON_DSC_ENCODER_CAP_SIZE])
> >       |                              ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_get_adjust_request_post_cursor':
> > drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is outside array bounds of 'const u8[6]' {aka 'const unsigned char[6]'} [-Werror=array-bounds]
> >    59 |         return link_status[r - DP_LANE0_1_STATUS];
> >       |                ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
> > drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing 'link_status'
> >   147 | u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZE],
> >       |                                          ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> > 
> > I can't see what in the kspp tree suddenly brought this on, so I have
> > used the kspp tree from next-20220228 for today.
> > 
> > In case it matters: x86_64-linux-gnu-gcc (Debian 11.2.0-9) 11.2.0  
> 
> This is fixed in drm-misc:
> 
> https://cgit.freedesktop.org/drm/drm-misc/log/
> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d4da1f27396fb1dde079447a3612f4f512caed07
> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a2151490cc6c57b368d7974ffd447a8b36ade639
> 
> but I had to drop the fix from the for-next/kspp because the patched
> file got moved in drm-misc.

Well, I moved the kspp tree to the end of the -next merges today, so it
would be after all the drm changes.  Unfortunately, it still fails:

drivers/gpu/drm/dp/drm_dp.c: In function 'drm_dp_pcon_dsc_bpp_incr':
drivers/gpu/drm/dp/drm_dp.c:3214:28: error: array subscript 12 is outside array bounds of 'const u8[12]' {aka 'const unsigned char[12]'} [-Werror=array-bounds]
 3214 |         buf = pcon_dsc_dpcd[DP_PCON_DSC_BPP_INCR - DP_PCON_DSC_ENCODER];
      |               ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/dp/drm_dp.c:3210:39: note: while referencing 'pcon_dsc_dpcd'
 3210 | int drm_dp_pcon_dsc_bpp_incr(const u8 pcon_dsc_dpcd[DP_PCON_DSC_ENCODER_CAP_SIZE])
      |                              ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/dp/drm_dp.c: In function 'drm_dp_get_adjust_request_post_cursor':
drivers/gpu/drm/dp/drm_dp.c:60:27: error: array subscript 10 is outside array bounds of 'const u8[6]' {aka 'const unsigned char[6]'} [-Werror=array-bounds]
   60 |         return link_status[r - DP_LANE0_1_STATUS];
      |                ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/dp/drm_dp.c:211:51: note: while referencing 'link_status'
  211 | u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZE],
      |                                          ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Presumably the above fixes have not hit the drm trees in linux-next yet
:-(

So, I used the version of your tree from next-20220228 again.  This
does not get the above, but does produce the error reported in
https://lore.kernel.org/lkml/20220225164231.904173-1-broonie@kernel.org/
(I was wondering why that went away today) for which I have applied the
patch in
https://lore.kernel.org/all/20220228081421.1504213-1-hsinyi@chromium.org/

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Tue, Mar 01, 2022 at 09:27:30AM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_pcon_dsc_bpp_incr':
> drivers/gpu/drm/drm_dp_helper.c:3130:28: error: array subscript 12 is outside array bounds of 'const u8[12]' {aka 'const unsigned char[12]'} [-Werror=array-bounds]
>  3130 |         buf = pcon_dsc_dpcd[DP_PCON_DSC_BPP_INCR - DP_PCON_DSC_ENCODER];
>       |               ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/drm_dp_helper.c:3126:39: note: while referencing 'pcon_dsc_dpcd'
>  3126 | int drm_dp_pcon_dsc_bpp_incr(const u8 pcon_dsc_dpcd[DP_PCON_DSC_ENCODER_CAP_SIZE])
>       |                              ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_get_adjust_request_post_cursor':
> drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is outside array bounds of 'const u8[6]' {aka 'const unsigned char[6]'} [-Werror=array-bounds]
>    59 |         return link_status[r - DP_LANE0_1_STATUS];
>       |                ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing 'link_status'
>   147 | u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZE],
>       |                                          ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> I can't see what in the kspp tree suddenly brought this on, so I have
> used the kspp tree from next-20220228 for today.
> 
> In case it matters: x86_64-linux-gnu-gcc (Debian 11.2.0-9) 11.2.0

This is fixed in drm-misc:

https://cgit.freedesktop.org/drm/drm-misc/log/
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d4da1f27396fb1dde079447a3612f4f512caed07
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a2151490cc6c57b368d7974ffd447a8b36ade639

but I had to drop the fix from the for-next/kspp because the patched
file got moved in drm-misc.

I don't know how to best deal with this case, which is: "add new Makefile
flag" and "carry fixes that have been ignored for 2 weeks" followed
by a later "fix got picked up now" change. Currently I've still been
carrying them is a separate tree that is merged with for-next/kspp,
so the warning doesn't appear like above, but that requires that the
tree it applies to doesn't change out from under it. :P

-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1674 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_pcon_dsc_bpp_incr':
drivers/gpu/drm/drm_dp_helper.c:3130:28: error: array subscript 12 is outside array bounds of 'const u8[12]' {aka 'const unsigned char[12]'} [-Werror=array-bounds]
 3130 |         buf = pcon_dsc_dpcd[DP_PCON_DSC_BPP_INCR - DP_PCON_DSC_ENCODER];
      |               ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/drm_dp_helper.c:3126:39: note: while referencing 'pcon_dsc_dpcd'
 3126 | int drm_dp_pcon_dsc_bpp_incr(const u8 pcon_dsc_dpcd[DP_PCON_DSC_ENCODER_CAP_SIZE])
      |                              ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_get_adjust_request_post_cursor':
drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is outside array bounds of 'const u8[6]' {aka 'const unsigned char[6]'} [-Werror=array-bounds]
   59 |         return link_status[r - DP_LANE0_1_STATUS];
      |                ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing 'link_status'
  147 | u8 drm_dp_get_adjust_request_post_cursor(const u8 link_status[DP_LINK_STATUS_SIZE],
      |                                          ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

I can't see what in the kspp tree suddenly brought this on, so I have
used the kspp tree from next-20220228 for today.

In case it matters: x86_64-linux-gnu-gcc (Debian 11.2.0-9) 11.2.0
-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Tue, Feb 08, 2022 at 03:42:18PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> [Cc'ing the scsi maintainers.  Sorry I should have done that sooner]
> 
> On Mon, 24 Jan 2022 19:43:44 -0800 Kees Cook <keescook@chromium.org> wrote:
> >
> > On Tue, Jan 25, 2022 at 02:24:30PM +1100, Stephen Rothwell wrote:
> > > 
> > > After merging the kspp tree, today's linux-next build (powerpc
> > > allyesconfig) failed like this:
> > > 
> > > 
> > > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c: In function 'ibmvscsis_send_messages':
> > > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1934:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
> > >  1934 |                                         crq->valid = VALID_CMD_RESP_EL;
> > >       |                                            ^~
> > > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
> > >  1875 |         u64 msg_hi = 0;
> > >       |             ^~~~~~
> > > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1935:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
> > >  1935 |                                         crq->format = cmd->rsp.format;
> > >       |                                            ^~
> > > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
> > >  1875 |         u64 msg_hi = 0;
> > >       |             ^~~~~~
> > > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1938:52: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
> > >  1938 |                                                 crq->status = VIOSRP_ADAPTER_FAIL;
> > >       |                                                    ^~
> > > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
> > >  1875 |         u64 msg_hi = 0;
> > >       |             ^~~~~~
> > > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1940:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
> > >  1940 |                                         crq->IU_length = cpu_to_be16(cmd->rsp.len);
> > >       |                                            ^~
> > > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
> > >  1875 |         u64 msg_hi = 0;
> > >       |             ^~~~~~
> > > 
> > > Exposed by commit
> > > 
> > >   4ba545781e20 ("Makefile: Enable -Warray-bounds")
> > > 
> > > Probably introduced by commit
> > > 
> > >   88a678bbc34c ("ibmvscsis: Initial commit of IBM VSCSI Tgt Driver")
> > > 
> > > I applied the following hack for now:
> > > 
> > > From: Stephen Rothwell <sfr@canb.auug.org.au>
> > > Date: Tue, 25 Jan 2022 14:18:36 +1100
> > > Subject: [PATCH] scsi: hack for building with -Warray-bounds
> > > 
> > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > ---
> > >  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> > > index 61f06f6885a5..89fcf98c61c3 100644
> > > --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> > > +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> > > @@ -1872,11 +1872,11 @@ static void srp_snd_msg_failed(struct scsi_info *vscsi, long rc)
> > >   */
> > >  static void ibmvscsis_send_messages(struct scsi_info *vscsi)
> > >  {
> > > -	u64 msg_hi = 0;
> > > +	u64 msg_hi[2] = { };
> > >  	/* note do not attempt to access the IU_data_ptr with this pointer
> > >  	 * it is not valid
> > >  	 */
> > > -	struct viosrp_crq *crq = (struct viosrp_crq *)&msg_hi;
> > > +	struct viosrp_crq *crq = (struct viosrp_crq *)msg_hi;
> > >  	struct ibmvscsis_cmd *cmd, *nxt;
> > >  	long rc = ADAPT_SUCCESS;
> > >  	bool retry = false;
> > > @@ -1940,7 +1940,7 @@ static void ibmvscsis_send_messages(struct scsi_info *vscsi)
> > >  					crq->IU_length = cpu_to_be16(cmd->rsp.len);
> > >  
> > >  					rc = h_send_crq(vscsi->dma_dev->unit_address,
> > > -							be64_to_cpu(msg_hi),
> > > +							be64_to_cpu(msg_hi[0]),
> > >  							be64_to_cpu(cmd->rsp.tag));
> > >  
> > >  					dev_dbg(&vscsi->dev, "send_messages: cmd %p, tag 0x%llx, rc %ld\n",  
> > 
> > This looks correct to me. struct viosrp_crq is 16 bytes wide. The only
> > suggestion I might make would be either avoid the bare '2':
> > 
> > 	u64 msg_hi[sizeof(struct viosrp_crq) / sizeof(u64)] = { };
> > 
> > or adjust struct viosrp_crq so the casting isn't needed at all:
> > 
> > 
> > truct viosrp_crq {
> > 	union {
> > 		u64 hi;
> > 		struct {
> > 		        u8 valid;               /* used by RPA */
> > 		        u8 format;              /* SCSI vs out-of-band */
> > 		        u8 reserved;
> > 		        u8 status;              /* non-scsi failure? (e.g. DMA failure) */
> > 		        __be16 timeout;         /* in seconds */
> > 		        __be16 IU_length;       /* in bytes */
> > 		};
> > 	};
> >         __be64 IU_data_ptr;     /* the TCE for transferring data */
> > };
> > 
> > 	struct viosrp_crq crq = { };
> > 	...
> >  			rc = h_send_crq(vscsi->dma_dev->unit_address,
> > 					be64_to_cpu(crq.hi),
> > 					be64_to_cpu(cmd->rsp.tag));
> > 
> > 
> > 
> 
> Has there been any progress on this?
> 
> Commit 88a678bbc34c mentioned above was merged in v4.8-rc1.

Thanks for the reminder! I've sent this now:

https://lore.kernel.org/lkml/20220208061231.3429486-1-keescook@chromium.org


-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 5402 bytes --]

Hi all,

[Cc'ing the scsi maintainers.  Sorry I should have done that sooner]

On Mon, 24 Jan 2022 19:43:44 -0800 Kees Cook <keescook@chromium.org> wrote:
>
> On Tue, Jan 25, 2022 at 02:24:30PM +1100, Stephen Rothwell wrote:
> > 
> > After merging the kspp tree, today's linux-next build (powerpc
> > allyesconfig) failed like this:
> > 
> > 
> > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c: In function 'ibmvscsis_send_messages':
> > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1934:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
> >  1934 |                                         crq->valid = VALID_CMD_RESP_EL;
> >       |                                            ^~
> > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
> >  1875 |         u64 msg_hi = 0;
> >       |             ^~~~~~
> > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1935:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
> >  1935 |                                         crq->format = cmd->rsp.format;
> >       |                                            ^~
> > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
> >  1875 |         u64 msg_hi = 0;
> >       |             ^~~~~~
> > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1938:52: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
> >  1938 |                                                 crq->status = VIOSRP_ADAPTER_FAIL;
> >       |                                                    ^~
> > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
> >  1875 |         u64 msg_hi = 0;
> >       |             ^~~~~~
> > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1940:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
> >  1940 |                                         crq->IU_length = cpu_to_be16(cmd->rsp.len);
> >       |                                            ^~
> > drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
> >  1875 |         u64 msg_hi = 0;
> >       |             ^~~~~~
> > 
> > Exposed by commit
> > 
> >   4ba545781e20 ("Makefile: Enable -Warray-bounds")
> > 
> > Probably introduced by commit
> > 
> >   88a678bbc34c ("ibmvscsis: Initial commit of IBM VSCSI Tgt Driver")
> > 
> > I applied the following hack for now:
> > 
> > From: Stephen Rothwell <sfr@canb.auug.org.au>
> > Date: Tue, 25 Jan 2022 14:18:36 +1100
> > Subject: [PATCH] scsi: hack for building with -Warray-bounds
> > 
> > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > ---
> >  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> > index 61f06f6885a5..89fcf98c61c3 100644
> > --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> > +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> > @@ -1872,11 +1872,11 @@ static void srp_snd_msg_failed(struct scsi_info *vscsi, long rc)
> >   */
> >  static void ibmvscsis_send_messages(struct scsi_info *vscsi)
> >  {
> > -	u64 msg_hi = 0;
> > +	u64 msg_hi[2] = { };
> >  	/* note do not attempt to access the IU_data_ptr with this pointer
> >  	 * it is not valid
> >  	 */
> > -	struct viosrp_crq *crq = (struct viosrp_crq *)&msg_hi;
> > +	struct viosrp_crq *crq = (struct viosrp_crq *)msg_hi;
> >  	struct ibmvscsis_cmd *cmd, *nxt;
> >  	long rc = ADAPT_SUCCESS;
> >  	bool retry = false;
> > @@ -1940,7 +1940,7 @@ static void ibmvscsis_send_messages(struct scsi_info *vscsi)
> >  					crq->IU_length = cpu_to_be16(cmd->rsp.len);
> >  
> >  					rc = h_send_crq(vscsi->dma_dev->unit_address,
> > -							be64_to_cpu(msg_hi),
> > +							be64_to_cpu(msg_hi[0]),
> >  							be64_to_cpu(cmd->rsp.tag));
> >  
> >  					dev_dbg(&vscsi->dev, "send_messages: cmd %p, tag 0x%llx, rc %ld\n",  
> 
> This looks correct to me. struct viosrp_crq is 16 bytes wide. The only
> suggestion I might make would be either avoid the bare '2':
> 
> 	u64 msg_hi[sizeof(struct viosrp_crq) / sizeof(u64)] = { };
> 
> or adjust struct viosrp_crq so the casting isn't needed at all:
> 
> 
> truct viosrp_crq {
> 	union {
> 		u64 hi;
> 		struct {
> 		        u8 valid;               /* used by RPA */
> 		        u8 format;              /* SCSI vs out-of-band */
> 		        u8 reserved;
> 		        u8 status;              /* non-scsi failure? (e.g. DMA failure) */
> 		        __be16 timeout;         /* in seconds */
> 		        __be16 IU_length;       /* in bytes */
> 		};
> 	};
>         __be64 IU_data_ptr;     /* the TCE for transferring data */
> };
> 
> 	struct viosrp_crq crq = { };
> 	...
>  			rc = h_send_crq(vscsi->dma_dev->unit_address,
> 					be64_to_cpu(crq.hi),
> 					be64_to_cpu(cmd->rsp.tag));
> 
> 
> 

Has there been any progress on this?

Commit 88a678bbc34c mentioned above was merged in v4.8-rc1.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Mon, Jan 31, 2022 at 11:10:19AM -0800, Saeed Mahameed wrote:
> On 30 Jan 22:19, Kees Cook wrote:
> > On Mon, Jan 31, 2022 at 03:59:32PM +1100, Stephen Rothwell wrote:
> > > On Sun, 30 Jan 2022 20:04:00 -0800 Kees Cook <keescook@chromium.org> wrote:
> > > > https://lore.kernel.org/linux-hardening/20220124172242.2410996-1-keescook@chromium.org/

Fixes: 34802a42b352 ("net/mlx5e: Do not modify the TX SKB")

> > > > https://lore.kernel.org/linux-hardening/20220124172028.2410761-1-keescook@chromium.org/

Fixes: b5503b994ed5 ("net/mlx5e: XDP TX forwarding support")

> > > yeah, neither has made it yet.  However, it would not have helped as I
> > > am merging the kspp tree very early so that new bugs get fixed in the
> > > trees that introduce them.  These 2 are in Linus tree (for a long time)
> > > and so it would be better if these fixes went int the net tree and then
> > > Linus' tree as bug fixes.
> 
> I need proper fixes tags to submit to net, can you provide ?

Sure! See above.

> another option is to use a shared branch with those fixes and pull it to
> both net-next and kspp.

It's just a handful left now, so I think we'll avoid this for now.

Thanks!

-- 
Kees Cook

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

[Saeed Mahameed]

On 30 Jan 22:19, Kees Cook wrote:
>On Mon, Jan 31, 2022 at 03:59:32PM +1100, Stephen Rothwell wrote:
>> On Sun, 30 Jan 2022 20:04:00 -0800 Kees Cook <keescook@chromium.org> wrote:
>> >
>> > This should be fixed by:
>> > https://lore.kernel.org/linux-hardening/20220124172242.2410996-1-keescook@chromium.org/
>> > (I was expecting this to be in netdev by now.)
>> >
>> > This should be fixed in:
>> > https://lore.kernel.org/linux-hardening/20220124172028.2410761-1-keescook@chromium.org/
>> > (Again, this was expected to be in netdev by now.)
>> >
>>
>> yeah, neither has made it yet.  However, it would not have helped as I
>> am merging the kspp tree very early so that new bugs get fixed in the
>> trees that introduce them.  These 2 are in Linus tree (for a long time)
>> and so it would be better if these fixes went int the net tree and then
>> Linus' tree as bug fixes.
>

I need proper fixes tags to submit to net, can you provide ? 

another option is to use a shared branch with those fixes and pull it to
both net-next and kspp.

let me know what option do you prefer.

-Saeed.

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

[Kees Cook]

On Mon, Jan 31, 2022 at 03:59:32PM +1100, Stephen Rothwell wrote:
> On Sun, 30 Jan 2022 20:04:00 -0800 Kees Cook <keescook@chromium.org> wrote:
> >
> > This should be fixed by:
> > https://lore.kernel.org/linux-hardening/20220124172242.2410996-1-keescook@chromium.org/
> > (I was expecting this to be in netdev by now.)
> > 
> > This should be fixed in:
> > https://lore.kernel.org/linux-hardening/20220124172028.2410761-1-keescook@chromium.org/
> > (Again, this was expected to be in netdev by now.)
> > 
> 
> yeah, neither has made it yet.  However, it would not have helped as I
> am merging the kspp tree very early so that new bugs get fixed in the
> trees that introduce them.  These 2 are in Linus tree (for a long time)
> and so it would be better if these fixes went int the net tree and then
> Linus' tree as bug fixes.

Okay, understood. I'll leave them in my tree until I see them duplicated
in their respective trees (or in Linus's tree).

> I will use these in the merge of the kspp tree from tomorrow until they
> appear earlier (or you could put them back in your tree).  Cross tree
> dependencies are a pain :-(   BTW Linus would have the same problem I
> am having if he merges your tree during the merge window before he
> merges the net-next tree ...

Yeah, I'm used to going "late" in the -rc1 from these kinds of things. :)

Thanks!

-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]

Hi Kees,

On Sun, 30 Jan 2022 20:04:00 -0800 Kees Cook <keescook@chromium.org> wrote:
>
> This should be fixed by:
> https://lore.kernel.org/linux-hardening/20220124172242.2410996-1-keescook@chromium.org/
> (I was expecting this to be in netdev by now.)
> 
> This should be fixed in:
> https://lore.kernel.org/linux-hardening/20220124172028.2410761-1-keescook@chromium.org/
> (Again, this was expected to be in netdev by now.)
> 

yeah, neither has made it yet.  However, it would not have helped as I
am merging the kspp tree very early so that new bugs get fixed in the
trees that introduce them.  These 2 are in Linus tree (for a long time)
and so it would be better if these fixes went int the net tree and then
Linus' tree as bug fixes.

I will use these in the merge of the kspp tree from tomorrow until they
appear earlier (or you could put them back in your tree).  Cross tree
dependencies are a pain :-(   BTW Linus would have the same problem I
am having if he merges your tree during the merge window before he
merges the net-next tree ...

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Mon, Jan 31, 2022 at 10:09:54AM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (x86_64
> 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 arch/x86/include/asm/cpumask.h:5,
>                  from arch/x86/include/asm/msr.h:11,
>                  from arch/x86/include/asm/processor.h:22,
>                  from arch/x86/include/asm/timex.h:5,
>                  from include/linux/timex.h:65,
>                  from include/linux/time32.h:13,
>                  from include/linux/time.h:60,
>                  from include/linux/skbuff.h:15,
>                  from include/linux/tcp.h:17,
>                  from drivers/net/ethernet/mellanox/mlx5/core/en_tx.c:33:
> In function 'fortify_memcpy_chk',
>     inlined from 'mlx5e_insert_vlan' at drivers/net/ethernet/mellanox/mlx5/core/en_tx.c:211:2,
>     inlined from 'mlx5e_sq_xmit_wqe' at drivers/net/ethernet/mellanox/mlx5/core/en_tx.c:496:4:
> include/linux/fortify-string.h:325:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>   325 |                         __write_overflow_field(p_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This should be fixed by:
https://lore.kernel.org/linux-hardening/20220124172242.2410996-1-keescook@chromium.org/
(I was expecting this to be in netdev by now.)

> In file included from include/linux/string.h:253,
>                  from include/linux/bitmap.h:11,
>                  from include/linux/cpumask.h:12,
>                  from arch/x86/include/asm/cpumask.h:5,
>                  from arch/x86/include/asm/msr.h:11,
>                  from arch/x86/include/asm/processor.h:22,
>                  from arch/x86/include/asm/timex.h:5,
>                  from include/linux/timex.h:65,
>                  from include/linux/time32.h:13,
>                  from include/linux/time.h:60,
>                  from include/linux/ktime.h:24,
>                  from include/linux/timer.h:6,
>                  from include/linux/netdevice.h:24,
>                  from include/trace/events/xdp.h:8,
>                  from include/linux/bpf_trace.h:5,
>                  from drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c:33:
> In function 'fortify_memcpy_chk',
>     inlined from 'mlx5e_xmit_xdp_frame' at drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c:344:3:
> include/linux/fortify-string.h:325:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>   325 |                         __write_overflow_field(p_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This should be fixed in:
https://lore.kernel.org/linux-hardening/20220124172028.2410761-1-keescook@chromium.org/
(Again, this was expected to be in netdev by now.)

> Caused by various commits in the mlx5 driver interacting with the new
> memcpy checking.
> 
> I have disabled CONFIG_MLX5_CORE_EN for today while this gets sorted
> out properly.

Thanks!

-Kees

-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 613 bytes --]

Hi Herbert,

On Mon, 31 Jan 2022 11:20:01 +1100 Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Mon, Jan 31, 2022 at 11:15:31AM +1100, Stephen Rothwell wrote:
> >
> > Are you intending to send that to Linus soon (as it fixes a commit that
> > was merged in v5.17-rc1)?  I would have expected to see it in the
> > crypto-current tree
> > (git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git#master)
> > in which case it would have been merged before the kspp tree.  
> 
> You're right Stephen.  I will push this to the crypto tree.

Thanks.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Herbert Xu]

On Mon, Jan 31, 2022 at 11:15:31AM +1100, Stephen Rothwell wrote:
>
> Are you intending to send that to Linus soon (as it fixes a commit that
> was merged in v5.17-rc1)?  I would have expected to see it in the
> crypto-current tree
> (git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git#master)
> in which case it would have been merged before the kspp tree.

You're right Stephen.  I will push this to the crypto tree.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

Hi Herbert,

On Mon, 31 Jan 2022 10:24:09 +1100 Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Mon, Jan 31, 2022 at 09:34:06AM +1100, Stephen Rothwell wrote:
> > Hi all,
> > 
> > After merging the kspp tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:  
> 
> This should be fixed if you merge the latest cryptodev tree as
> it includes the following fix:
> 
> commit f0a26ee8e1f8bda99f1e0050292de928cec17f92
> Author: Kees Cook <keescook@chromium.org>
> Date:   Wed Jan 12 12:38:11 2022 -0800
> 
>     crypto: octeontx2 - Avoid stack variable overflow

Are you intending to send that to Linus soon (as it fixes a commit that
was merged in v5.17-rc1)?  I would have expected to see it in the
crypto-current tree
(git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git#master)
in which case it would have been merged before the kspp tree.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Herbert Xu]

On Mon, Jan 31, 2022 at 09:34:06AM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (x86_64
> allmodconfig) failed like this:

This should be fixed if you merge the latest cryptodev tree as
it includes the following fix:

commit f0a26ee8e1f8bda99f1e0050292de928cec17f92
Author: Kees Cook <keescook@chromium.org>
Date:   Wed Jan 12 12:38:11 2022 -0800

    crypto: octeontx2 - Avoid stack variable overflow

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2982 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
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 arch/x86/include/asm/cpumask.h:5,
                 from arch/x86/include/asm/msr.h:11,
                 from arch/x86/include/asm/processor.h:22,
                 from arch/x86/include/asm/timex.h:5,
                 from include/linux/timex.h:65,
                 from include/linux/time32.h:13,
                 from include/linux/time.h:60,
                 from include/linux/skbuff.h:15,
                 from include/linux/tcp.h:17,
                 from drivers/net/ethernet/mellanox/mlx5/core/en_tx.c:33:
In function 'fortify_memcpy_chk',
    inlined from 'mlx5e_insert_vlan' at drivers/net/ethernet/mellanox/mlx5/core/en_tx.c:211:2,
    inlined from 'mlx5e_sq_xmit_wqe' at drivers/net/ethernet/mellanox/mlx5/core/en_tx.c:496:4:
include/linux/fortify-string.h:325:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
  325 |                         __write_overflow_field(p_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/string.h:253,
                 from include/linux/bitmap.h:11,
                 from include/linux/cpumask.h:12,
                 from arch/x86/include/asm/cpumask.h:5,
                 from arch/x86/include/asm/msr.h:11,
                 from arch/x86/include/asm/processor.h:22,
                 from arch/x86/include/asm/timex.h:5,
                 from include/linux/timex.h:65,
                 from include/linux/time32.h:13,
                 from include/linux/time.h:60,
                 from include/linux/ktime.h:24,
                 from include/linux/timer.h:6,
                 from include/linux/netdevice.h:24,
                 from include/trace/events/xdp.h:8,
                 from include/linux/bpf_trace.h:5,
                 from drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c:33:
In function 'fortify_memcpy_chk',
    inlined from 'mlx5e_xmit_xdp_frame' at drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c:344:3:
include/linux/fortify-string.h:325:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
  325 |                         __write_overflow_field(p_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Caused by various commits in the mlx5 driver interacting with the new
memcpy checking.

I have disabled CONFIG_MLX5_CORE_EN for today while this gets sorted
out properly.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3149 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
allmodconfig) failed like this:

In file included from include/linux/printk.h:555,
                 from include/asm-generic/bug.h:22,
                 from arch/x86/include/asm/bug.h:84,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/firmware.h:7,
                 from drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c:5:
drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c: In function 'otx2_cpt_print_uc_dbg_info':
include/linux/dynamic_debug.h:162:33: error: array subscript 4 is above array bounds of 'u32[4]' {aka 'unsigned int[4]'} [-Werror=array-bounds]
  162 |         _dynamic_func_call(fmt, __dynamic_pr_debug,             \
      |                                 ^
include/linux/dynamic_debug.h:134:17: note: in definition of macro '__dynamic_func_call'
  134 |                 func(&id, ##__VA_ARGS__);               \
      |                 ^~~~
include/linux/dynamic_debug.h:162:9: note: in expansion of macro '_dynamic_func_call'
  162 |         _dynamic_func_call(fmt, __dynamic_pr_debug,             \
      |         ^~~~~~~~~~~~~~~~~~
include/linux/printk.h:570:9: note: in expansion of macro 'dynamic_pr_debug'
  570 |         dynamic_pr_debug(fmt, ##__VA_ARGS__)
      |         ^~~~~~~~~~~~~~~~
drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c:1798:41: note: in expansion of macro 'pr_debug'
 1798 |                                         pr_debug("Mask: %8.8x %8.8x %8.8x %8.8x %8.8x",
      |                                         ^~~~~~~~
drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c:1756:13: note: while referencing 'mask'
 1756 |         u32 mask[4];
      |             ^~~~

Caused by commit

  d9d7749773e8 ("crypto: octeontx2 - add apis for custom engine groups")

from Linus' tree interacting with commit

  a8712a32665f ("Makefile: Enable -Warray-bounds")

from the kspp tree.

I have applied the following patch for now.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 31 Jan 2022 09:28:12 +1100
Subject: [PATCH] fix up for otx2_cptpf_ucode.c out of bound reference

Fixes: d9d7749773e8 ("crypto: octeontx2 - add apis for custom engine groups")
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c b/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c
index 4c8ebdf671ca..e990405c7f33 100644
--- a/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c
+++ b/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c
@@ -1753,7 +1753,7 @@ void otx2_cpt_print_uc_dbg_info(struct otx2_cptpf_dev *cptpf)
 	char engs_info[2 * OTX2_CPT_NAME_LENGTH];
 	struct otx2_cpt_eng_grp_info *grp;
 	struct otx2_cpt_engs_rsvd *engs;
-	u32 mask[4];
+	u32 mask[5];
 	int i, j;
 
 	pr_debug("Engine groups global info");
-- 
2.34.1

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Steven Rostedt]

On Wed, 26 Jan 2022 11:41:38 -0800
Beau Belgrave <beaub@linux.microsoft.com> wrote:

> Right, and on the user side we will not know the start of the "data" /
> fixed offset without carrying back that information somehow, due to the
> common header size not being a fixed length forever.

Good point! The data is not exported to user space via the format files
like the __ref_loc field is, so there's no guarantee of knowing where data
is.

Let's leave it as is.

-- Steve

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

[Beau Belgrave]

On Tue, Jan 25, 2022 at 10:26:11PM -0500, Steven Rostedt wrote:
> On Tue, 25 Jan 2022 19:18:53 -0800
> Kees Cook <keescook@chromium.org> wrote:
> 
> > On Tue, Jan 25, 2022 at 08:16:34PM -0500, Steven Rostedt wrote:
> > > On Wed, 26 Jan 2022 09:35:38 +0900
> > > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > >   
> > > > I think Kees' idea seems better. If you and Beau are good, I will update
> > > > the macros for __rel_loc. (This requires to change some user-space
> > > > application which Beau is making too.)  

I like the simplicity of using offset from the start of data, however,
we would have to save the start of data somewhere for this. The user
trace processing side won't know the size of the common header (unless I
missed something elsewhere got added).

Due to this, I would *prefer* to have the offset still be the offset
from the __rel_loc entry. That makes it absolutely clear to both user and
kernel without requiring any additional information where to find it.

> > > 
> > > If Beau is OK with it, I'm OK with it too. I need to release a new version
> > > of libtraceevent anyway, and I can make the update for that too.
> > > 
> > > Who's adding the patch (if Beau says it's OK), you or Kees?  
> > 
> > I don't know anything about libtraceevent, so hopefully not me! :) The
> > patches Masami and I already sent fix the warning, so I leave it you
> > y'all to decide if you want to make the internals a bit simpler.
> > 
> 
> 
> Thinking about this more. I may just take both your patches, and leave it
> as an offset from the location of the descriptor. It's closer to the
> meaning of "relative" than doing it as a fixed offset from data.
> 
> And then no other patches or user space needs to be changed.
> 

Right, and on the user side we will not know the start of the "data" /
fixed offset without carrying back that information somehow, due to the
common header size not being a fixed length forever.

> Thanks,
> 
> -- Steve

Thanks,
-Beau

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

[Steven Rostedt]

On Wed, 26 Jan 2022 12:52:52 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> > On Wed, 26 Jan 2022 09:35:38 +0900
> > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >   
> > > I think Kees' idea seems better. If you and Beau are good, I will update
> > > the macros for __rel_loc. (This requires to change some user-space
> > > application which Beau is making too.)  
> > 
> > If Beau is OK with it, I'm OK with it too. I need to release a new version
> > of libtraceevent anyway, and I can make the update for that too.
> > 
> > Who's adding the patch (if Beau says it's OK), you or Kees?  
> 
> This will update the __rel_loc spec, so I'll do it :)

As I replied to the other email, I don't think we want to do this.

As "rel" means relative, I think it makes more sense to keep it as is, and
not have it just be from the data section. That may be even more confusing.

But I'll wait for Beau to answer.

Note, I added both your and Kees patches to my queue and I'm currently
testing it. You can see what I'm testing here:

  https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git/log/?h=ftrace/core

-- Steve

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

[Masami Hiramatsu]

On Tue, 25 Jan 2022 20:16:34 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 26 Jan 2022 09:35:38 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > I think Kees' idea seems better. If you and Beau are good, I will update
> > the macros for __rel_loc. (This requires to change some user-space
> > application which Beau is making too.)
> 
> If Beau is OK with it, I'm OK with it too. I need to release a new version
> of libtraceevent anyway, and I can make the update for that too.
> 
> Who's adding the patch (if Beau says it's OK), you or Kees?

This will update the __rel_loc spec, so I'll do it :)

Thank you,
> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

[Steven Rostedt]

On Tue, 25 Jan 2022 19:18:53 -0800
Kees Cook <keescook@chromium.org> wrote:

> On Tue, Jan 25, 2022 at 08:16:34PM -0500, Steven Rostedt wrote:
> > On Wed, 26 Jan 2022 09:35:38 +0900
> > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >   
> > > I think Kees' idea seems better. If you and Beau are good, I will update
> > > the macros for __rel_loc. (This requires to change some user-space
> > > application which Beau is making too.)  
> > 
> > If Beau is OK with it, I'm OK with it too. I need to release a new version
> > of libtraceevent anyway, and I can make the update for that too.
> > 
> > Who's adding the patch (if Beau says it's OK), you or Kees?  
> 
> I don't know anything about libtraceevent, so hopefully not me! :) The
> patches Masami and I already sent fix the warning, so I leave it you
> y'all to decide if you want to make the internals a bit simpler.
> 


Thinking about this more. I may just take both your patches, and leave it
as an offset from the location of the descriptor. It's closer to the
meaning of "relative" than doing it as a fixed offset from data.

And then no other patches or user space needs to be changed.

Thanks,

-- Steve

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

[Kees Cook]

On Tue, Jan 25, 2022 at 08:16:34PM -0500, Steven Rostedt wrote:
> On Wed, 26 Jan 2022 09:35:38 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > I think Kees' idea seems better. If you and Beau are good, I will update
> > the macros for __rel_loc. (This requires to change some user-space
> > application which Beau is making too.)
> 
> If Beau is OK with it, I'm OK with it too. I need to release a new version
> of libtraceevent anyway, and I can make the update for that too.
> 
> Who's adding the patch (if Beau says it's OK), you or Kees?

I don't know anything about libtraceevent, so hopefully not me! :) The
patches Masami and I already sent fix the warning, so I leave it you
y'all to decide if you want to make the internals a bit simpler.

-- 
Kees Cook

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

[Kees Cook]

On Wed, Jan 26, 2022 at 09:35:38AM +0900, Masami Hiramatsu wrote:
> Hi Steve,
> (Ccing Beau)
> 
> On Tue, 25 Jan 2022 17:21:14 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Tue, 25 Jan 2022 14:07:14 -0800
> > Kees Cook <keescook@chromium.org> wrote:
> > 
> > > > The tstruct is the TP_STRUCT__entry() and for each __rel_dynamic_array() or
> > > > __dynamic_array(), the __data_size gets updated and saved into the
> > > > __data_offsets that holds where each item is.
> > > > 
> > > > The rel versions sets the offset from its location to the data, where as
> > > > the non rel versions sets the offset from the beginning of the event to the
> > > > data.  
> > > 
> > > Could this just be
> > > 
> > > #define __get_rel_dynamic_array(field) \
> > > 	((void *)(&__entry->data[__entry->__rel_loc_##field & 0xffff])
> > > 
> > > ?
> > 
> > This is currently user space defined. But since the only user of the rel_*
> > version hasn't been upstreamed yet, we could change it. But it also
> > requires changing libtraceevent as it depends on this code too.
> 
> I think Kees' idea seems better. If you and Beau are good, I will update
> the macros for __rel_loc. (This requires to change some user-space
> application which Beau is making too.)
> 
> > 
> > I'm surprised that it doesn't break with the __get_dynamic_array()
> > versions, or is that because it's based off of __entry?
> 
> I think so. Gcc seems to check the size of the data structure where the
> original base address points.

Right, and it's pretty good these days at navigating through casts and
inlines, etc, which is appreciated in all the cases where it has found
real bugs. :)

The "offset from __entry" solution works as well as "offset from
__entry->data", so I don't care which is used. If "offset from data"
makes more sense for this API, yeah, I guess change it now while it's
possible. :)

Thanks for helping track this down!

-- 
Kees Cook

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

[Steven Rostedt]

On Wed, 26 Jan 2022 09:35:38 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> I think Kees' idea seems better. If you and Beau are good, I will update
> the macros for __rel_loc. (This requires to change some user-space
> application which Beau is making too.)

If Beau is OK with it, I'm OK with it too. I need to release a new version
of libtraceevent anyway, and I can make the update for that too.

Who's adding the patch (if Beau says it's OK), you or Kees?

-- Steve

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

[Masami Hiramatsu]

Hi Steve,
(Ccing Beau)

On Tue, 25 Jan 2022 17:21:14 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 25 Jan 2022 14:07:14 -0800
> Kees Cook <keescook@chromium.org> wrote:
> 
> > > The tstruct is the TP_STRUCT__entry() and for each __rel_dynamic_array() or
> > > __dynamic_array(), the __data_size gets updated and saved into the
> > > __data_offsets that holds where each item is.
> > > 
> > > The rel versions sets the offset from its location to the data, where as
> > > the non rel versions sets the offset from the beginning of the event to the
> > > data.  
> > 
> > Could this just be
> > 
> > #define __get_rel_dynamic_array(field) \
> > 	((void *)(&__entry->data[__entry->__rel_loc_##field & 0xffff])
> > 
> > ?
> 
> This is currently user space defined. But since the only user of the rel_*
> version hasn't been upstreamed yet, we could change it. But it also
> requires changing libtraceevent as it depends on this code too.

I think Kees' idea seems better. If you and Beau are good, I will update
the macros for __rel_loc. (This requires to change some user-space
application which Beau is making too.)

> 
> I'm surprised that it doesn't break with the __get_dynamic_array()
> versions, or is that because it's based off of __entry?

I think so. Gcc seems to check the size of the data structure where the
original base address points.

Thank you,

> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

[Steven Rostedt]

On Tue, 25 Jan 2022 14:07:14 -0800
Kees Cook <keescook@chromium.org> wrote:

> > The tstruct is the TP_STRUCT__entry() and for each __rel_dynamic_array() or
> > __dynamic_array(), the __data_size gets updated and saved into the
> > __data_offsets that holds where each item is.
> > 
> > The rel versions sets the offset from its location to the data, where as
> > the non rel versions sets the offset from the beginning of the event to the
> > data.  
> 
> Could this just be
> 
> #define __get_rel_dynamic_array(field) \
> 	((void *)(&__entry->data[__entry->__rel_loc_##field & 0xffff])
> 
> ?

This is currently user space defined. But since the only user of the rel_*
version hasn't been upstreamed yet, we could change it. But it also
requires changing libtraceevent as it depends on this code too.

I'm surprised that it doesn't break with the __get_dynamic_array()
versions, or is that because it's based off of __entry?

-- Steve

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

[Kees Cook]

On Tue, Jan 25, 2022 at 04:28:59PM -0500, Steven Rostedt wrote:
> On Tue, 25 Jan 2022 16:23:26 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > > >  #undef __get_rel_dynamic_array
> > > > -#define __get_rel_dynamic_array(field)	\
> > > > -		((void *)(&__entry->__rel_loc_##field) +	\
> > > > -		 sizeof(__entry->__rel_loc_##field) +		\
> > > > +#define __get_rel_dynamic_array(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    
> > > 
> 
> I also do not like the the inconsistency between
> sizeof(__entry->__rel_loc_##field) and sizeof(u32) that is used in the
> calculation part:
> 
> 
> #define __rel_dynamic_array(type, item, len)				\
> 	__item_length = (len) * sizeof(type);				\
> 	__data_offsets->item = __data_size +				\
> 			       offsetof(typeof(*entry), __data) -	\
> 			       offsetof(typeof(*entry), __rel_loc_##item) -	\
> 			       sizeof(u32);				\
> 	__data_offsets->item |= __item_length << 16;			\
> 	__data_size += __item_length;
> 
> Why is one using sizeof(u32) and the other using the size of the field?

It might make more sense to calculate everything as an offset within
__data[] instead of from the start of __entry. The patch I sent just did
in perf.h exactly what Masami did in trace_event.h. That worked. I had
an earlier version that did this horrible thing which could probably be
significantly improved, since I just subtract the offset of __data:

 #undef __get_rel_dynamic_array
 #define __get_rel_dynamic_array(field) \
-		((void *)(&__entry->__rel_loc_##field) +        \
-		 sizeof(__entry->__rel_loc_##field) +           \
-		 (__entry->__rel_loc_##field & 0xffff))
+		((void *)&__entry->__data[			\
+		 offsetof(typeof(*__entry), __rel_loc_##field)	\
+		 + sizeof(__entry->__rel_loc_##field)		\
+		 + (__entry->__rel_loc_##field & 0xffff)	\
+		 - offsetof(typeof(*__entry), __data)		\
+		])

> Just to let you know what is happening. As dynamic elements of the trace
> event needs to be appended at the end of the event, the above macros are
> defined and then run through the TRACE_EVENT() macro, where the
> TP_STRUCT__entry() is parsed to calculate where each item will be for that
> event.
> 
> static inline notrace int trace_event_get_offsets_##call(		\
> 	struct trace_event_data_offsets_##call *__data_offsets, proto)	\
> {									\
> 	int __data_size = 0;						\
> 	int __maybe_unused __item_length;				\
> 	struct trace_event_raw_##call __maybe_unused *entry;		\
> 									\
> 	tstruct;							\
> 									\
> 	return __data_size;						\
> }
> 
> 
> The tstruct is the TP_STRUCT__entry() and for each __rel_dynamic_array() or
> __dynamic_array(), the __data_size gets updated and saved into the
> __data_offsets that holds where each item is.
> 
> The rel versions sets the offset from its location to the data, where as
> the non rel versions sets the offset from the beginning of the event to the
> data.

Could this just be

#define __get_rel_dynamic_array(field) \
	((void *)(&__entry->data[__entry->__rel_loc_##field & 0xffff])

?

-- 
Kees Cook

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

[Kees Cook]

On Tue, Jan 25, 2022 at 04:23:26PM -0500, Steven Rostedt wrote:
> On Tue, 25 Jan 2022 12:57:24 -0800
> Kees Cook <keescook@chromium.org> wrote:
> 
> > On Tue, Jan 25, 2022 at 11:31:54PM +0900, Masami Hiramatsu wrote:
> > > On Tue, 25 Jan 2022 22:27:32 +0900
> > > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > >   
> > > > > /*
> > > > >  * 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) (?)  
> > > > 
> > > > Ah, I got it. Yes, that's right. __data_loc() will access the data
> > > > from the __entry, but the __rel_loc() points the same address from
> > > > the encoded field ("__rel_loc_foo" in this case) itself.
> > > > This is introduced for the user application event, which doesn't
> > > > know the actual __entry size because the __entry includes some
> > > > kernel internal defined fields.
> > > >   
> > > > > But if this is true, I would imagine there would be plenty of other
> > > > > warnings? I'm currently stumped.  
> > > > 
> > > > That is because __rel_loc is used only in the sample code in the kernel
> > > > for testing. Other use-cases comes from user-space.
> > > > Hmm, can we skip this boundary check for this example?  
> > > 
> > > If the -Warray-bounds determines the destination array size from
> > > the type of given pointer, we can just change the macro as below;
> > > 
> > > #define __get_rel_dynamic_array(field) 
> > > 			((void *)__entry +                                 \
> > > 			 offsetof(typeof(*__entry), __rel_loc_##field) +   \
> > > 			 sizeof(__entry->__rel_loc_##field) +              \
> > > 			 (__entry->__rel_loc_##field & 0xffff))
> > > 
> > > This must works same as __get_dynamic_array() macro.
> > > 
> > > Could you try this patch?
> > > 
> > > From 2982ba01367ec1f746a4f128512436e5325a7f9d Mon Sep 17 00:00:00 2001
> > > From: Masami Hiramatsu <mhiramat@kernel.org>
> > > Date: Tue, 25 Jan 2022 23:19:30 +0900
> > > Subject: [PATCH] tracing: Avoid -Warray-bounds warning for __rel_loc macro
> > > 
> > > Since -Warray-bounds checks the destination size from the
> > > type of given pointer, __assign_rel_str() macro gets warned
> > > because it passes the pointer to the 'u32' field instead of
> > > 'trace_event_raw_*' data structure.
> > > Pass the data address calculated from the 'trace_event_raw_*'
> > > instead of 'u32' __rel_loc field.
> > > 
> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > Cc: Kees Cook <keescook@chromium.org>
> > > ---
> > >  include/trace/trace_events.h | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
> > > index 8c6f7c433518..65d927e059d3 100644
> > > --- a/include/trace/trace_events.h
> > > +++ b/include/trace/trace_events.h
> > > @@ -318,9 +318,10 @@ TRACE_MAKE_SYSTEM_STR();
> > >  #define __get_str(field) ((char *)__get_dynamic_array(field))
> > >  
> > >  #undef __get_rel_dynamic_array
> > > -#define __get_rel_dynamic_array(field)	\
> > > -		((void *)(&__entry->__rel_loc_##field) +	\
> > > -		 sizeof(__entry->__rel_loc_##field) +		\
> > > +#define __get_rel_dynamic_array(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  
> > 
> > This patch doesn't silence the warning, but now that I see the shape of
> > things more clearly, let me see if I can find the right combo.
> 
> Hmm, could the zero size array cause an issues here. That is, does this
> help?
> 
> diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
> index 65d927e059d3..3d29919045af 100644
> --- a/include/trace/trace_events.h
> +++ b/include/trace/trace_events.h
> @@ -128,7 +128,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;

I changed this too, just to future-proof it, and in an attempt to base
the address off of __data[] (which turned out not to be needed).

It turns out that there was still a version of the __get_rel_dyanmic_array
in perf.h that was the actual culprit.

-- 
Kees Cook

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

[Steven Rostedt]

On Tue, 25 Jan 2022 16:23:26 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> > >  #undef __get_rel_dynamic_array
> > > -#define __get_rel_dynamic_array(field)	\
> > > -		((void *)(&__entry->__rel_loc_##field) +	\
> > > -		 sizeof(__entry->__rel_loc_##field) +		\
> > > +#define __get_rel_dynamic_array(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    
> > 

I also do not like the the inconsistency between
sizeof(__entry->__rel_loc_##field) and sizeof(u32) that is used in the
calculation part:


#define __rel_dynamic_array(type, item, len)				\
	__item_length = (len) * sizeof(type);				\
	__data_offsets->item = __data_size +				\
			       offsetof(typeof(*entry), __data) -	\
			       offsetof(typeof(*entry), __rel_loc_##item) -	\
			       sizeof(u32);				\
	__data_offsets->item |= __item_length << 16;			\
	__data_size += __item_length;

Why is one using sizeof(u32) and the other using the size of the field?

Just to let you know what is happening. As dynamic elements of the trace
event needs to be appended at the end of the event, the above macros are
defined and then run through the TRACE_EVENT() macro, where the
TP_STRUCT__entry() is parsed to calculate where each item will be for that
event.

static inline notrace int trace_event_get_offsets_##call(		\
	struct trace_event_data_offsets_##call *__data_offsets, proto)	\
{									\
	int __data_size = 0;						\
	int __maybe_unused __item_length;				\
	struct trace_event_raw_##call __maybe_unused *entry;		\
									\
	tstruct;							\
									\
	return __data_size;						\
}


The tstruct is the TP_STRUCT__entry() and for each __rel_dynamic_array() or
__dynamic_array(), the __data_size gets updated and saved into the
__data_offsets that holds where each item is.

The rel versions sets the offset from its location to the data, where as
the non rel versions sets the offset from the beginning of the event to the
data.

-- Steve

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

[Steven Rostedt]

On Tue, 25 Jan 2022 12:57:24 -0800
Kees Cook <keescook@chromium.org> wrote:

> On Tue, Jan 25, 2022 at 11:31:54PM +0900, Masami Hiramatsu wrote:
> > On Tue, 25 Jan 2022 22:27:32 +0900
> > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >   
> > > > /*
> > > >  * 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) (?)  
> > > 
> > > Ah, I got it. Yes, that's right. __data_loc() will access the data
> > > from the __entry, but the __rel_loc() points the same address from
> > > the encoded field ("__rel_loc_foo" in this case) itself.
> > > This is introduced for the user application event, which doesn't
> > > know the actual __entry size because the __entry includes some
> > > kernel internal defined fields.
> > >   
> > > > But if this is true, I would imagine there would be plenty of other
> > > > warnings? I'm currently stumped.  
> > > 
> > > That is because __rel_loc is used only in the sample code in the kernel
> > > for testing. Other use-cases comes from user-space.
> > > Hmm, can we skip this boundary check for this example?  
> > 
> > If the -Warray-bounds determines the destination array size from
> > the type of given pointer, we can just change the macro as below;
> > 
> > #define __get_rel_dynamic_array(field) 
> > 			((void *)__entry +                                 \
> > 			 offsetof(typeof(*__entry), __rel_loc_##field) +   \
> > 			 sizeof(__entry->__rel_loc_##field) +              \
> > 			 (__entry->__rel_loc_##field & 0xffff))
> > 
> > This must works same as __get_dynamic_array() macro.
> > 
> > Could you try this patch?
> > 
> > From 2982ba01367ec1f746a4f128512436e5325a7f9d Mon Sep 17 00:00:00 2001
> > From: Masami Hiramatsu <mhiramat@kernel.org>
> > Date: Tue, 25 Jan 2022 23:19:30 +0900
> > Subject: [PATCH] tracing: Avoid -Warray-bounds warning for __rel_loc macro
> > 
> > Since -Warray-bounds checks the destination size from the
> > type of given pointer, __assign_rel_str() macro gets warned
> > because it passes the pointer to the 'u32' field instead of
> > 'trace_event_raw_*' data structure.
> > Pass the data address calculated from the 'trace_event_raw_*'
> > instead of 'u32' __rel_loc field.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Cc: Kees Cook <keescook@chromium.org>
> > ---
> >  include/trace/trace_events.h | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
> > index 8c6f7c433518..65d927e059d3 100644
> > --- a/include/trace/trace_events.h
> > +++ b/include/trace/trace_events.h
> > @@ -318,9 +318,10 @@ TRACE_MAKE_SYSTEM_STR();
> >  #define __get_str(field) ((char *)__get_dynamic_array(field))
> >  
> >  #undef __get_rel_dynamic_array
> > -#define __get_rel_dynamic_array(field)	\
> > -		((void *)(&__entry->__rel_loc_##field) +	\
> > -		 sizeof(__entry->__rel_loc_##field) +		\
> > +#define __get_rel_dynamic_array(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  
> 
> This patch doesn't silence the warning, but now that I see the shape of
> things more clearly, let me see if I can find the right combo.

Hmm, could the zero size array cause an issues here. That is, does this
help?

diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
index 65d927e059d3..3d29919045af 100644
--- a/include/trace/trace_events.h
+++ b/include/trace/trace_events.h
@@ -128,7 +128,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;


-- Steve

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

[Kees Cook]

On Tue, Jan 25, 2022 at 11:31:54PM +0900, Masami Hiramatsu wrote:
> On Tue, 25 Jan 2022 22:27:32 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > > /*
> > >  * 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) (?)
> > 
> > Ah, I got it. Yes, that's right. __data_loc() will access the data
> > from the __entry, but the __rel_loc() points the same address from
> > the encoded field ("__rel_loc_foo" in this case) itself.
> > This is introduced for the user application event, which doesn't
> > know the actual __entry size because the __entry includes some
> > kernel internal defined fields.
> > 
> > > But if this is true, I would imagine there would be plenty of other
> > > warnings? I'm currently stumped.
> > 
> > That is because __rel_loc is used only in the sample code in the kernel
> > for testing. Other use-cases comes from user-space.
> > Hmm, can we skip this boundary check for this example?
> 
> If the -Warray-bounds determines the destination array size from
> the type of given pointer, we can just change the macro as below;
> 
> #define __get_rel_dynamic_array(field) 
> 			((void *)__entry +                                 \
> 			 offsetof(typeof(*__entry), __rel_loc_##field) +   \
> 			 sizeof(__entry->__rel_loc_##field) +              \
> 			 (__entry->__rel_loc_##field & 0xffff))
> 
> This must works same as __get_dynamic_array() macro.
> 
> Could you try this patch?
> 
> From 2982ba01367ec1f746a4f128512436e5325a7f9d Mon Sep 17 00:00:00 2001
> From: Masami Hiramatsu <mhiramat@kernel.org>
> Date: Tue, 25 Jan 2022 23:19:30 +0900
> Subject: [PATCH] tracing: Avoid -Warray-bounds warning for __rel_loc macro
> 
> Since -Warray-bounds checks the destination size from the
> type of given pointer, __assign_rel_str() macro gets warned
> because it passes the pointer to the 'u32' field instead of
> 'trace_event_raw_*' data structure.
> Pass the data address calculated from the 'trace_event_raw_*'
> instead of 'u32' __rel_loc field.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Kees Cook <keescook@chromium.org>
> ---
>  include/trace/trace_events.h | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
> index 8c6f7c433518..65d927e059d3 100644
> --- a/include/trace/trace_events.h
> +++ b/include/trace/trace_events.h
> @@ -318,9 +318,10 @@ TRACE_MAKE_SYSTEM_STR();
>  #define __get_str(field) ((char *)__get_dynamic_array(field))
>  
>  #undef __get_rel_dynamic_array
> -#define __get_rel_dynamic_array(field)	\
> -		((void *)(&__entry->__rel_loc_##field) +	\
> -		 sizeof(__entry->__rel_loc_##field) +		\
> +#define __get_rel_dynamic_array(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

This patch doesn't silence the warning, but now that I see the shape of
things more clearly, let me see if I can find the right combo.

Thanks!

-Kees

-- 
Kees Cook

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

[Steven Rostedt]

On Tue, 25 Jan 2022 09:54:14 -0800
Kees Cook <keescook@chromium.org> wrote:

> Thanks, I'll give this a spin. I need to reproduce sfr's warning first,
> but now that I've fetched next-20220125, it should be easy. *famous last
> words*

I'll start my tests with this patch, and when/if you test it, please
send a Tested-by tag.

Thanks,

-- Steve

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

[Kees Cook]

On Tue, Jan 25, 2022 at 11:31:54PM +0900, Masami Hiramatsu wrote:
> On Tue, 25 Jan 2022 22:27:32 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > > /*
> > >  * 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) (?)
> > 
> > Ah, I got it. Yes, that's right. __data_loc() will access the data
> > from the __entry, but the __rel_loc() points the same address from
> > the encoded field ("__rel_loc_foo" in this case) itself.
> > This is introduced for the user application event, which doesn't
> > know the actual __entry size because the __entry includes some
> > kernel internal defined fields.
> > 
> > > But if this is true, I would imagine there would be plenty of other
> > > warnings? I'm currently stumped.
> > 
> > That is because __rel_loc is used only in the sample code in the kernel
> > for testing. Other use-cases comes from user-space.
> > Hmm, can we skip this boundary check for this example?
> 
> If the -Warray-bounds determines the destination array size from
> the type of given pointer, we can just change the macro as below;
> 
> #define __get_rel_dynamic_array(field) 
> 			((void *)__entry +                                 \
> 			 offsetof(typeof(*__entry), __rel_loc_##field) +   \
> 			 sizeof(__entry->__rel_loc_##field) +              \
> 			 (__entry->__rel_loc_##field & 0xffff))
> 
> This must works same as __get_dynamic_array() macro.
> 
> Could you try this patch?

Thanks, I'll give this a spin. I need to reproduce sfr's warning first,
but now that I've fetched next-20220125, it should be easy. *famous last
words*

-- 
Kees Cook

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

[Kees Cook]

On Tue, Jan 25, 2022 at 09:01:52AM -0500, Steven Rostedt wrote:
> On Tue, 25 Jan 2022 22:27:32 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > > But if this is true, I would imagine there would be plenty of other
> > > warnings? I'm currently stumped.  
> > 
> > That is because __rel_loc is used only in the sample code in the kernel
> > for testing. Other use-cases comes from user-space.
> > Hmm, can we skip this boundary check for this example?
> 
> Is this only checked when __CHECKER__ is defined? If so, would this work?

__CHECKER__ is only for sparse. This is from re-enabling -Warray-bounds
for gcc.

-- 
Kees Cook

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

[Steven Rostedt]

On Tue, 25 Jan 2022 23:31:54 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Could you try this patch?
> 
> >From 2982ba01367ec1f746a4f128512436e5325a7f9d Mon Sep 17 00:00:00 2001  
> From: Masami Hiramatsu <mhiramat@kernel.org>
> Date: Tue, 25 Jan 2022 23:19:30 +0900
> Subject: [PATCH] tracing: Avoid -Warray-bounds warning for __rel_loc macro
> 
> Since -Warray-bounds checks the destination size from the
> type of given pointer, __assign_rel_str() macro gets warned
> because it passes the pointer to the 'u32' field instead of
> 'trace_event_raw_*' data structure.
> Pass the data address calculated from the 'trace_event_raw_*'
> instead of 'u32' __rel_loc field.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Kees Cook <keescook@chromium.org>

If this works, I'll add it instead of the one that just turns it off it
__CHECKER__ is defined.

Can you please let me know if it fixes the issue for you. I have a test run
waiting on it.

Thanks,

-- Steve

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

[Masami Hiramatsu]

On Tue, 25 Jan 2022 22:27:32 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> > /*
> >  * 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) (?)
> 
> Ah, I got it. Yes, that's right. __data_loc() will access the data
> from the __entry, but the __rel_loc() points the same address from
> the encoded field ("__rel_loc_foo" in this case) itself.
> This is introduced for the user application event, which doesn't
> know the actual __entry size because the __entry includes some
> kernel internal defined fields.
> 
> > But if this is true, I would imagine there would be plenty of other
> > warnings? I'm currently stumped.
> 
> That is because __rel_loc is used only in the sample code in the kernel
> for testing. Other use-cases comes from user-space.
> Hmm, can we skip this boundary check for this example?

If the -Warray-bounds determines the destination array size from
the type of given pointer, we can just change the macro as below;

#define __get_rel_dynamic_array(field) 
			((void *)__entry +                                 \
			 offsetof(typeof(*__entry), __rel_loc_##field) +   \
			 sizeof(__entry->__rel_loc_##field) +              \
			 (__entry->__rel_loc_##field & 0xffff))

This must works same as __get_dynamic_array() macro.

Could you try this patch?

From 2982ba01367ec1f746a4f128512436e5325a7f9d Mon Sep 17 00:00:00 2001
From: Masami Hiramatsu <mhiramat@kernel.org>
Date: Tue, 25 Jan 2022 23:19:30 +0900
Subject: [PATCH] tracing: Avoid -Warray-bounds warning for __rel_loc macro

Since -Warray-bounds checks the destination size from the
type of given pointer, __assign_rel_str() macro gets warned
because it passes the pointer to the 'u32' field instead of
'trace_event_raw_*' data structure.
Pass the data address calculated from the 'trace_event_raw_*'
instead of 'u32' __rel_loc field.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Kees Cook <keescook@chromium.org>
---
 include/trace/trace_events.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
index 8c6f7c433518..65d927e059d3 100644
--- a/include/trace/trace_events.h
+++ b/include/trace/trace_events.h
@@ -318,9 +318,10 @@ TRACE_MAKE_SYSTEM_STR();
 #define __get_str(field) ((char *)__get_dynamic_array(field))
 
 #undef __get_rel_dynamic_array
-#define __get_rel_dynamic_array(field)	\
-		((void *)(&__entry->__rel_loc_##field) +	\
-		 sizeof(__entry->__rel_loc_##field) +		\
+#define __get_rel_dynamic_array(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
-- 
2.25.1


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

[David Sterba]

On Tue, Jan 25, 2022 at 11:57:57AM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (x86_64
> 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 arch/x86/include/asm/cpumask.h:5,
>                  from arch/x86/include/asm/msr.h:11,
>                  from arch/x86/include/asm/processor.h:22,
>                  from arch/x86/include/asm/cpufeature.h:5,
>                  from arch/x86/include/asm/thread_info.h:53,
>                  from include/linux/thread_info.h:60,
>                  from arch/x86/include/asm/preempt.h:7,
>                  from include/linux/preempt.h:78,
>                  from include/linux/spinlock.h:55,
>                  from include/linux/wait.h:9,
>                  from include/linux/mempool.h:8,
>                  from include/linux/bio.h:8,
>                  from fs/btrfs/ioctl.c:7:
> In function 'fortify_memcpy_chk',
>     inlined from 'btrfs_ioctl_encoded_write' at fs/btrfs/ioctl.c:5082:3:
> include/linux/fortify-string.h:316:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>   316 |                         __write_overflow_field(p_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/fortify-string.h:324:25: error: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
>   324 |                         __read_overflow2_field(q_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Caused by commit
> 
>   602670289b69 ("fortify: Detect struct member overflows in memcpy() at compile-time")
> 
> interacting with commit
> 
>   504e1ebb6316 ("btrfs: add BTRFS_IOC_ENCODED_WRITE")
> 
> from the btrfs tree.
> 
> I applied the following hack:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 25 Jan 2022 11:47:17 +1100
> Subject: [PATCH] fix up for "btrfs: add BTRFS_IOC_ENCODED_WRITE"
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  fs/btrfs/ioctl.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 73ad918a05a9..d34620034f8e 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -5079,9 +5079,14 @@ static int btrfs_ioctl_encoded_write(struct file *file, void __user *argp,
>  		}
>  		args.iov = compat_ptr(args32.iov);
>  		args.iovcnt = args32.iovcnt;
> -		memcpy(&args.offset, &args32.offset,
> -		       sizeof(args) -
> -		       offsetof(struct btrfs_ioctl_encoded_io_args, offset));
> +		args.offset = args32.offset;
> +		args.flags = args32.flags;
> +		args.len = args32.len;
> +		args.unencoded_len = args32.unencoded_len;
> +		args.unencoded_offset = args32.unencoded_offset;
> +		args.compression = args32.compression;
> +		args.encryption = args32.encryption;
> +		memcpy(args.reserved, args32.reserved, sizeof(args.reserved));
>  #else
>  		return -ENOTTY;
>  #endif

Thanks, the patchset is still in progress so I'll apply this a fixup
until the patch gets updated.

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

[Steven Rostedt]

On Tue, 25 Jan 2022 22:27:32 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> > But if this is true, I would imagine there would be plenty of other
> > warnings? I'm currently stumped.  
> 
> That is because __rel_loc is used only in the sample code in the kernel
> for testing. Other use-cases comes from user-space.
> Hmm, can we skip this boundary check for this example?

Is this only checked when __CHECKER__ is defined? If so, would this work?

-- Steve

diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h
index cbbbb83beced..cdc68e0f9ef9 100644
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -506,6 +506,8 @@ DEFINE_EVENT_PRINT(foo_template, foo_with_template_print,
 	TP_ARGS(foo, bar),
 	TP_printk("bar %s %d", __get_str(foo), __entry->bar));
 
+/* rel_loc is for user defined events, the checker will trigger bugs on it */
+#ifndef __CHECKER__
 /*
  * There are yet another __rel_loc dynamic data attribute. If you
  * use __rel_dynamic_array() and __rel_string() etc. macros, you
@@ -539,6 +541,10 @@ TRACE_EVENT(foo_rel_loc,
 	TP_printk("foo_rel_loc %s, %d, %s", __get_rel_str(foo), __entry->bar,
 		  __get_rel_bitmask(bitmask))
 );
+#else
+#define trace_foo_rel_loc(foo, bar, mask) do { } while (0)
+#endif /* __CHECKER __ */
+
 #endif
 
 /***** NOTICE! The #if protection ends here. *****/

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

[Masami Hiramatsu]

Hi Kees,

On Mon, 24 Jan 2022 23:44:05 -0800
Kees Cook <keescook@chromium.org> wrote:

> 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:

The __entry is the trace-event entry on the trace ring_buffer. This
reserved an entry (area) on the ring_buffer and fills it with
given traced data. "__rel_loc_foo" is the a field on the entry,
which type is u32. This should be something like this.

struct {
  ...
  u32 __rel_loc_foo;
  ...
} *__entry;

> 
> /*
>  * 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) (?)

Ah, I got it. Yes, that's right. __data_loc() will access the data
from the __entry, but the __rel_loc() points the same address from
the encoded field ("__rel_loc_foo" in this case) itself.
This is introduced for the user application event, which doesn't
know the actual __entry size because the __entry includes some
kernel internal defined fields.

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

That is because __rel_loc is used only in the sample code in the kernel
for testing. Other use-cases comes from user-space.
Hmm, can we skip this boundary check for this example?

Thank you,

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


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

[Kees Cook]

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

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]

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]
   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.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Tue, Jan 25, 2022 at 02:24:30PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
> 
> 
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c: In function 'ibmvscsis_send_messages':
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1934:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
>  1934 |                                         crq->valid = VALID_CMD_RESP_EL;
>       |                                            ^~
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
>  1875 |         u64 msg_hi = 0;
>       |             ^~~~~~
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1935:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
>  1935 |                                         crq->format = cmd->rsp.format;
>       |                                            ^~
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
>  1875 |         u64 msg_hi = 0;
>       |             ^~~~~~
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1938:52: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
>  1938 |                                                 crq->status = VIOSRP_ADAPTER_FAIL;
>       |                                                    ^~
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
>  1875 |         u64 msg_hi = 0;
>       |             ^~~~~~
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1940:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
>  1940 |                                         crq->IU_length = cpu_to_be16(cmd->rsp.len);
>       |                                            ^~
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
>  1875 |         u64 msg_hi = 0;
>       |             ^~~~~~
> 
> Exposed by commit
> 
>   4ba545781e20 ("Makefile: Enable -Warray-bounds")
> 
> Probably introduced by commit
> 
>   88a678bbc34c ("ibmvscsis: Initial commit of IBM VSCSI Tgt Driver")
> 
> I applied the following hack for now:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 25 Jan 2022 14:18:36 +1100
> Subject: [PATCH] scsi: hack for building with -Warray-bounds
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> index 61f06f6885a5..89fcf98c61c3 100644
> --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> @@ -1872,11 +1872,11 @@ static void srp_snd_msg_failed(struct scsi_info *vscsi, long rc)
>   */
>  static void ibmvscsis_send_messages(struct scsi_info *vscsi)
>  {
> -	u64 msg_hi = 0;
> +	u64 msg_hi[2] = { };
>  	/* note do not attempt to access the IU_data_ptr with this pointer
>  	 * it is not valid
>  	 */
> -	struct viosrp_crq *crq = (struct viosrp_crq *)&msg_hi;
> +	struct viosrp_crq *crq = (struct viosrp_crq *)msg_hi;
>  	struct ibmvscsis_cmd *cmd, *nxt;
>  	long rc = ADAPT_SUCCESS;
>  	bool retry = false;
> @@ -1940,7 +1940,7 @@ static void ibmvscsis_send_messages(struct scsi_info *vscsi)
>  					crq->IU_length = cpu_to_be16(cmd->rsp.len);
>  
>  					rc = h_send_crq(vscsi->dma_dev->unit_address,
> -							be64_to_cpu(msg_hi),
> +							be64_to_cpu(msg_hi[0]),
>  							be64_to_cpu(cmd->rsp.tag));
>  
>  					dev_dbg(&vscsi->dev, "send_messages: cmd %p, tag 0x%llx, rc %ld\n",

This looks correct to me. struct viosrp_crq is 16 bytes wide. The only
suggestion I might make would be either avoid the bare '2':

	u64 msg_hi[sizeof(struct viosrp_crq) / sizeof(u64)] = { };

or adjust struct viosrp_crq so the casting isn't needed at all:


truct viosrp_crq {
	union {
		u64 hi;
		struct {
		        u8 valid;               /* used by RPA */
		        u8 format;              /* SCSI vs out-of-band */
		        u8 reserved;
		        u8 status;              /* non-scsi failure? (e.g. DMA failure) */
		        __be16 timeout;         /* in seconds */
		        __be16 IU_length;       /* in bytes */
		};
	};
        __be64 IU_data_ptr;     /* the TCE for transferring data */
};

	struct viosrp_crq crq = { };
	...
 			rc = h_send_crq(vscsi->dma_dev->unit_address,
					be64_to_cpu(crq.hi),
					be64_to_cpu(cmd->rsp.tag));



-- 
Kees Cook

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

[Kees Cook]

On Tue, Jan 25, 2022 at 11:57:57AM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (x86_64
> 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 arch/x86/include/asm/cpumask.h:5,
>                  from arch/x86/include/asm/msr.h:11,
>                  from arch/x86/include/asm/processor.h:22,
>                  from arch/x86/include/asm/cpufeature.h:5,
>                  from arch/x86/include/asm/thread_info.h:53,
>                  from include/linux/thread_info.h:60,
>                  from arch/x86/include/asm/preempt.h:7,
>                  from include/linux/preempt.h:78,
>                  from include/linux/spinlock.h:55,
>                  from include/linux/wait.h:9,
>                  from include/linux/mempool.h:8,
>                  from include/linux/bio.h:8,
>                  from fs/btrfs/ioctl.c:7:
> In function 'fortify_memcpy_chk',
>     inlined from 'btrfs_ioctl_encoded_write' at fs/btrfs/ioctl.c:5082:3:
> include/linux/fortify-string.h:316:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>   316 |                         __write_overflow_field(p_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/fortify-string.h:324:25: error: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
>   324 |                         __read_overflow2_field(q_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Caused by commit
> 
>   602670289b69 ("fortify: Detect struct member overflows in memcpy() at compile-time")
> 
> interacting with commit
> 
>   504e1ebb6316 ("btrfs: add BTRFS_IOC_ENCODED_WRITE")
> 
> from the btrfs tree.

Thanks!

I found the btrfs patch here:
https://lore.kernel.org/all/ec08e6f559ab47b3300ca5a67e8fc984fd3f040f.1637179348.git.osandov@fb.com/

> 
> I applied the following hack:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 25 Jan 2022 11:47:17 +1100
> Subject: [PATCH] fix up for "btrfs: add BTRFS_IOC_ENCODED_WRITE"
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  fs/btrfs/ioctl.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 73ad918a05a9..d34620034f8e 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -5079,9 +5079,14 @@ static int btrfs_ioctl_encoded_write(struct file *file, void __user *argp,
>  		}
>  		args.iov = compat_ptr(args32.iov);
>  		args.iovcnt = args32.iovcnt;
> -		memcpy(&args.offset, &args32.offset,
> -		       sizeof(args) -
> -		       offsetof(struct btrfs_ioctl_encoded_io_args, offset));
> +		args.offset = args32.offset;
> +		args.flags = args32.flags;
> +		args.len = args32.len;
> +		args.unencoded_len = args32.unencoded_len;
> +		args.unencoded_offset = args32.unencoded_offset;
> +		args.compression = args32.compression;
> +		args.encryption = args32.encryption;
> +		memcpy(args.reserved, args32.reserved, sizeof(args.reserved));
>  #else
>  		return -ENOTTY;
>  #endif

I'll see if I can construct something shorter using struct_group().

-Kees

-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3834 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (powerpc
allyesconfig) failed like this:


drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c: In function 'ibmvscsis_send_messages':
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1934:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
 1934 |                                         crq->valid = VALID_CMD_RESP_EL;
      |                                            ^~
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
 1875 |         u64 msg_hi = 0;
      |             ^~~~~~
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1935:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
 1935 |                                         crq->format = cmd->rsp.format;
      |                                            ^~
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
 1875 |         u64 msg_hi = 0;
      |             ^~~~~~
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1938:52: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
 1938 |                                                 crq->status = VIOSRP_ADAPTER_FAIL;
      |                                                    ^~
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
 1875 |         u64 msg_hi = 0;
      |             ^~~~~~
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1940:44: error: array subscript 'struct viosrp_crq[0]' is partly outside array bounds of 'u64[1]' {aka 'long long unsigned int[1]'} [-Werror=array-bounds]
 1940 |                                         crq->IU_length = cpu_to_be16(cmd->rsp.len);
      |                                            ^~
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1875:13: note: while referencing 'msg_hi'
 1875 |         u64 msg_hi = 0;
      |             ^~~~~~

Exposed by commit

  4ba545781e20 ("Makefile: Enable -Warray-bounds")

Probably introduced by commit

  88a678bbc34c ("ibmvscsis: Initial commit of IBM VSCSI Tgt Driver")

I applied the following hack for now:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 25 Jan 2022 14:18:36 +1100
Subject: [PATCH] scsi: hack for building with -Warray-bounds

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 61f06f6885a5..89fcf98c61c3 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1872,11 +1872,11 @@ static void srp_snd_msg_failed(struct scsi_info *vscsi, long rc)
  */
 static void ibmvscsis_send_messages(struct scsi_info *vscsi)
 {
-	u64 msg_hi = 0;
+	u64 msg_hi[2] = { };
 	/* note do not attempt to access the IU_data_ptr with this pointer
 	 * it is not valid
 	 */
-	struct viosrp_crq *crq = (struct viosrp_crq *)&msg_hi;
+	struct viosrp_crq *crq = (struct viosrp_crq *)msg_hi;
 	struct ibmvscsis_cmd *cmd, *nxt;
 	long rc = ADAPT_SUCCESS;
 	bool retry = false;
@@ -1940,7 +1940,7 @@ static void ibmvscsis_send_messages(struct scsi_info *vscsi)
 					crq->IU_length = cpu_to_be16(cmd->rsp.len);
 
 					rc = h_send_crq(vscsi->dma_dev->unit_address,
-							be64_to_cpu(msg_hi),
+							be64_to_cpu(msg_hi[0]),
 							be64_to_cpu(cmd->rsp.tag));
 
 					dev_dbg(&vscsi->dev, "send_messages: cmd %p, tag 0x%llx, rc %ld\n",
-- 
2.34.1

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Tue, Jan 25, 2022 at 02:02:21PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> On Tue, 25 Jan 2022 13:55:46 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > After merging the kspp tree, today's linux-next build (powerpc
> > allyesconfig) failed like this:
> > 
> > drivers/gpu/drm/kmb/kmb_plane.c: In function 'kmb_plane_atomic_disable':
> > drivers/gpu/drm/kmb/kmb_plane.c:165:34: error: array subscript 3 is above array bounds of 'struct layer_status[2]' [-Werror=array-bounds]
> >   165 |                 kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL2_ENABLE;
> >       |                 ~~~~~~~~~~~~~~~~~^~~~~~~~~~
> > In file included from drivers/gpu/drm/kmb/kmb_plane.c:17:
> > drivers/gpu/drm/kmb/kmb_drv.h:61:41: note: while referencing 'plane_status'
> >    61 |         struct layer_status             plane_status[KMB_MAX_PLANES];
> >       |                                         ^~~~~~~~~~~~
> > drivers/gpu/drm/kmb/kmb_plane.c:162:34: error: array subscript 2 is above array bounds of 'struct layer_status[2]' [-Werror=array-bounds]
> >   162 |                 kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL1_ENABLE;
> >       |                 ~~~~~~~~~~~~~~~~~^~~~~~~~~~
> > In file included from drivers/gpu/drm/kmb/kmb_plane.c:17:
> > drivers/gpu/drm/kmb/kmb_drv.h:61:41: note: while referencing 'plane_status'
> >    61 |         struct layer_status             plane_status[KMB_MAX_PLANES];
> >       |                                         ^~~~~~~~~~~~
> > 
> > Exposed by commit
> > 
> >   4ba545781e20 ("Makefile: Enable -Warray-bounds")
> > 
> > Originally introduced by commmit
> > 
> >   7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> > 
> > Maybe should have been fixed by
> > 
> >   c026565fe9be ("drm/kmb: Enable alpha blended second plane")

Yeah, that had fixed it at some point. :(

> > 
> > I have applied the following hack for now.
> 
> Actually, this one:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 25 Jan 2022 13:45:26 +1100
> Subject: [PATCH] hack for building with -Warray-bounds
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  drivers/gpu/drm/kmb/kmb_plane.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
> index 00404ba4126d..b54197920712 100644
> --- a/drivers/gpu/drm/kmb/kmb_plane.c
> +++ b/drivers/gpu/drm/kmb/kmb_plane.c
> @@ -158,12 +158,14 @@ static void kmb_plane_atomic_disable(struct drm_plane *plane,
>  	case LAYER_1:
>  		kmb->plane_status[plane_id].ctrl = LCD_CTRL_VL2_ENABLE;
>  		break;
> +#if 0
>  	case LAYER_2:
>  		kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL1_ENABLE;
>  		break;
>  	case LAYER_3:
>  		kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL2_ENABLE;
>  		break;
> +#endif
>  	}
>  
>  	kmb->plane_status[plane_id].disable = true;

Thanks!

The prior thread about this was here:
https://lore.kernel.org/lkml/20210825181807.1138053-1-keescook@chromium.org/

-Kees

-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2728 bytes --]

Hi all,

On Tue, 25 Jan 2022 13:55:46 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> After merging the kspp tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
> 
> drivers/gpu/drm/kmb/kmb_plane.c: In function 'kmb_plane_atomic_disable':
> drivers/gpu/drm/kmb/kmb_plane.c:165:34: error: array subscript 3 is above array bounds of 'struct layer_status[2]' [-Werror=array-bounds]
>   165 |                 kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL2_ENABLE;
>       |                 ~~~~~~~~~~~~~~~~~^~~~~~~~~~
> In file included from drivers/gpu/drm/kmb/kmb_plane.c:17:
> drivers/gpu/drm/kmb/kmb_drv.h:61:41: note: while referencing 'plane_status'
>    61 |         struct layer_status             plane_status[KMB_MAX_PLANES];
>       |                                         ^~~~~~~~~~~~
> drivers/gpu/drm/kmb/kmb_plane.c:162:34: error: array subscript 2 is above array bounds of 'struct layer_status[2]' [-Werror=array-bounds]
>   162 |                 kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL1_ENABLE;
>       |                 ~~~~~~~~~~~~~~~~~^~~~~~~~~~
> In file included from drivers/gpu/drm/kmb/kmb_plane.c:17:
> drivers/gpu/drm/kmb/kmb_drv.h:61:41: note: while referencing 'plane_status'
>    61 |         struct layer_status             plane_status[KMB_MAX_PLANES];
>       |                                         ^~~~~~~~~~~~
> 
> Exposed by commit
> 
>   4ba545781e20 ("Makefile: Enable -Warray-bounds")
> 
> Originally introduced by commmit
> 
>   7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")
> 
> Maybe should have been fixed by
> 
>   c026565fe9be ("drm/kmb: Enable alpha blended second plane")
> 
> I have applied the following hack for now.

Actually, this one:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 25 Jan 2022 13:45:26 +1100
Subject: [PATCH] hack for building with -Warray-bounds

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/gpu/drm/kmb/kmb_plane.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
index 00404ba4126d..b54197920712 100644
--- a/drivers/gpu/drm/kmb/kmb_plane.c
+++ b/drivers/gpu/drm/kmb/kmb_plane.c
@@ -158,12 +158,14 @@ static void kmb_plane_atomic_disable(struct drm_plane *plane,
 	case LAYER_1:
 		kmb->plane_status[plane_id].ctrl = LCD_CTRL_VL2_ENABLE;
 		break;
+#if 0
 	case LAYER_2:
 		kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL1_ENABLE;
 		break;
 	case LAYER_3:
 		kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL2_ENABLE;
 		break;
+#endif
 	}
 
 	kmb->plane_status[plane_id].disable = true;
-- 
2.34.1

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2466 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (powerpc
allyesconfig) failed like this:

drivers/gpu/drm/kmb/kmb_plane.c: In function 'kmb_plane_atomic_disable':
drivers/gpu/drm/kmb/kmb_plane.c:165:34: error: array subscript 3 is above array bounds of 'struct layer_status[2]' [-Werror=array-bounds]
  165 |                 kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL2_ENABLE;
      |                 ~~~~~~~~~~~~~~~~~^~~~~~~~~~
In file included from drivers/gpu/drm/kmb/kmb_plane.c:17:
drivers/gpu/drm/kmb/kmb_drv.h:61:41: note: while referencing 'plane_status'
   61 |         struct layer_status             plane_status[KMB_MAX_PLANES];
      |                                         ^~~~~~~~~~~~
drivers/gpu/drm/kmb/kmb_plane.c:162:34: error: array subscript 2 is above array bounds of 'struct layer_status[2]' [-Werror=array-bounds]
  162 |                 kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL1_ENABLE;
      |                 ~~~~~~~~~~~~~~~~~^~~~~~~~~~
In file included from drivers/gpu/drm/kmb/kmb_plane.c:17:
drivers/gpu/drm/kmb/kmb_drv.h:61:41: note: while referencing 'plane_status'
   61 |         struct layer_status             plane_status[KMB_MAX_PLANES];
      |                                         ^~~~~~~~~~~~

Exposed by commit

  4ba545781e20 ("Makefile: Enable -Warray-bounds")

Originally introduced by commmit

  7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display")

Maybe should have been fixed by

  c026565fe9be ("drm/kmb: Enable alpha blended second plane")

I have applied the following hack for now.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 25 Jan 2022 13:45:26 +1100
Subject: [PATCH] hack for building with -Warray-bounds

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/gpu/drm/kmb/kmb_plane.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
index 00404ba4126d..c12e900c6c93 100644
--- a/drivers/gpu/drm/kmb/kmb_plane.c
+++ b/drivers/gpu/drm/kmb/kmb_plane.c
@@ -161,9 +161,11 @@ static void kmb_plane_atomic_disable(struct drm_plane *plane,
 	case LAYER_2:
 		kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL1_ENABLE;
 		break;
+#if 0
 	case LAYER_3:
 		kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL2_ENABLE;
 		break;
+#endif
 	}
 
 	kmb->plane_status[plane_id].disable = true;
-- 
2.34.1

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3283 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
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 arch/x86/include/asm/cpumask.h:5,
                 from arch/x86/include/asm/msr.h:11,
                 from arch/x86/include/asm/processor.h:22,
                 from arch/x86/include/asm/cpufeature.h:5,
                 from arch/x86/include/asm/thread_info.h:53,
                 from include/linux/thread_info.h:60,
                 from arch/x86/include/asm/preempt.h:7,
                 from include/linux/preempt.h:78,
                 from include/linux/spinlock.h:55,
                 from include/linux/wait.h:9,
                 from include/linux/mempool.h:8,
                 from include/linux/bio.h:8,
                 from fs/btrfs/ioctl.c:7:
In function 'fortify_memcpy_chk',
    inlined from 'btrfs_ioctl_encoded_write' at fs/btrfs/ioctl.c:5082:3:
include/linux/fortify-string.h:316:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
  316 |                         __write_overflow_field(p_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/fortify-string.h:324:25: error: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
  324 |                         __read_overflow2_field(q_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Caused by commit

  602670289b69 ("fortify: Detect struct member overflows in memcpy() at compile-time")

interacting with commit

  504e1ebb6316 ("btrfs: add BTRFS_IOC_ENCODED_WRITE")

from the btrfs tree.

I applied the following hack:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 25 Jan 2022 11:47:17 +1100
Subject: [PATCH] fix up for "btrfs: add BTRFS_IOC_ENCODED_WRITE"

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/btrfs/ioctl.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 73ad918a05a9..d34620034f8e 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5079,9 +5079,14 @@ static int btrfs_ioctl_encoded_write(struct file *file, void __user *argp,
 		}
 		args.iov = compat_ptr(args32.iov);
 		args.iovcnt = args32.iovcnt;
-		memcpy(&args.offset, &args32.offset,
-		       sizeof(args) -
-		       offsetof(struct btrfs_ioctl_encoded_io_args, offset));
+		args.offset = args32.offset;
+		args.flags = args32.flags;
+		args.len = args32.len;
+		args.unencoded_len = args32.unencoded_len;
+		args.unencoded_offset = args32.unencoded_offset;
+		args.compression = args32.compression;
+		args.encryption = args32.encryption;
+		memcpy(args.reserved, args32.reserved, sizeof(args.reserved));
 #else
 		return -ENOTTY;
 #endif
-- 
2.34.1

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On September 15, 2021 8:34:33 PM PDT, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>Hi all,
>
>After merging the kspp tree, today's linux-next build (powerpc
>ppc64_defconfig) failed like this:
>
>In file included from include/linux/compiler_types.h:85,
>                 from <command-line>:
>include/linux/compiler-gcc.h:44: warning: "__compiletime_warning" redefined
>   44 | #define __compiletime_warning(message) __attribute__((__warning__(message)))
>      | 
>
>And many more.
>
>Caused by commit
>
>  0fc9374deb67 ("compiler_types.h: Remove __compiletime_object_size()")
>
>Something went badly wrong in the rebase :-(

Ieeeee! Thank you. Fixing...

-Kees

>
>I have used the kspp tree from next-20210916 for today.
>

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

In file included from include/linux/compiler_types.h:85,
                 from <command-line>:
include/linux/compiler-gcc.h:44: warning: "__compiletime_warning" redefined
   44 | #define __compiletime_warning(message) __attribute__((__warning__(message)))
      | 

And many more.

Caused by commit

  0fc9374deb67 ("compiler_types.h: Remove __compiletime_object_size()")

Something went badly wrong in the rebase :-(

I have used the kspp tree from next-20210916 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On August 26, 2021 12:52:05 AM PDT, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>Hi all,
>
>After merging the kspp tree, today's linux-next build (x86_64
>allmodconfig) failed like this:
>
>In file included from ./usr/include/linux/posix_types.h:5,
>                 from ./usr/include/linux/types.h:9,
>                 from ./usr/include/rdma/rdma_user_rxe.h:37,
>                 from <command-line>:32:
>./usr/include/rdma/rdma_user_rxe.h:134:24: error: unknown type name 'u8'
>  134 |   __DECLARE_FLEX_ARRAY(u8, inline_data);
>      |                        ^~
>./usr/include/linux/stddef.h:42:3: note: in definition of macro '__DECLARE_FLEX_ARRAY'
>   42 |   TYPE NAME[]; \
>      |   ^~~~

Thanks -- I've fixed this now. Typo in conversion. :)

-Kees

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 802 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
allmodconfig) failed like this:

In file included from ./usr/include/linux/posix_types.h:5,
                 from ./usr/include/linux/types.h:9,
                 from ./usr/include/rdma/rdma_user_rxe.h:37,
                 from <command-line>:32:
./usr/include/rdma/rdma_user_rxe.h:134:24: error: unknown type name 'u8'
  134 |   __DECLARE_FLEX_ARRAY(u8, inline_data);
      |                        ^~
./usr/include/linux/stddef.h:42:3: note: in definition of macro '__DECLARE_FLEX_ARRAY'
   42 |   TYPE NAME[]; \
      |   ^~~~

Caused by commit

  a908f2e5a6d0 ("treewide: Replace open-coded flex arrays in unions")

I have used the kspp tree from next-20210825 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[David Miller]

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 23 Jun 2020 13:51:34 +1000

> I have added the following merge fix patch.
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 23 Jun 2020 13:43:06 +1000
> Subject: [PATCH] net/core/devlink.c: remove new uninitialized_var() usage
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

Applied, thanks Stephen.

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

net/core/devlink.c: In function 'devlink_nl_port_function_attrs_put':
net/core/devlink.c:586:3: warning: parameter names (without types) in function declaration
  586 |   int uninitialized_var(hw_addr_len);
      |   ^~~
net/core/devlink.c:589:65: error: 'hw_addr_len' undeclared (first use in this function); did you mean 'hw_addr'?
  589 |   err = ops->port_function_hw_addr_get(devlink, port, hw_addr, &hw_addr_len, extack);
      |                                                                 ^~~~~~~~~~~
      |                                                                 hw_addr
net/core/devlink.c:589:65: note: each undeclared identifier is reported only once for each function it appears in

Caused by commit

  2e6d06799c15 ("compiler: Remove uninitialized_var() macro")

interacting with commit

  2a916ecc4056 ("net/devlink: Support querying hardware address of port function")

from the net-next tree.

I have added the following merge fix patch.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 23 Jun 2020 13:43:06 +1000
Subject: [PATCH] net/core/devlink.c: remove new uninitialized_var() usage

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 net/core/devlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 455998a57671..6ae36808c152 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -583,7 +583,7 @@ devlink_nl_port_function_attrs_put(struct sk_buff *msg, struct devlink_port *por
 
 	ops = devlink->ops;
 	if (ops->port_function_hw_addr_get) {
-		int uninitialized_var(hw_addr_len);
+		int hw_addr_len;
 		u8 hw_addr[MAX_ADDR_LEN];
 
 		err = ops->port_function_hw_addr_get(devlink, port, hw_addr, &hw_addr_len, extack);
-- 
2.27.0

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Sun, Jun 21, 2020 at 11:48:51PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (x86_64 allnoconfig)
> failed like this:
> 
> x86_64-linux-gnu-ld: fs/file.o: in function `__fd_install_received':
> file.c:(.text+0x1010): undefined reference to `sock_from_file'
> x86_64-linux-gnu-ld: file.c:(.text+0x104a): undefined reference to `sock_from_file'

Oh fun. Okay, that's the first use of sock_from_file() in core kernel
code. I will fix linux/net.h to include a NULL-returning static inline
for the CONFIG_NET=n case.

And I will add "allnoconfig" to my test workflow. :)

-- 
Kees Cook

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64 allnoconfig)
failed like this:

x86_64-linux-gnu-ld: fs/file.o: in function `__fd_install_received':
file.c:(.text+0x1010): undefined reference to `sock_from_file'
x86_64-linux-gnu-ld: file.c:(.text+0x104a): undefined reference to `sock_from_file'

Caused by commit

  d3868eea5cbc ("fs: Move __scm_install_fd() to __fd_install_received()")

I reverted these commits for today (from the breaking commit to the end
of the branch):

b29bb87cbb0a selftests/seccomp: Test SECCOMP_IOCTL_NOTIF_ADDFD
af35c3c6a9a5 seccomp: Introduce addfd ioctl to seccomp user notifier
50ca89d3a4fb fs: Expand __fd_install_received() to accept fd
f533d1758f02 pidfd: Replace open-coded partial fd_install_received()
4ab6bcc3ad3b fs: Add fd_install_received() wrapper for __fd_install_received()
d3868eea5cbc fs: Move __scm_install_fd() to __fd_install_received()

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 316 bytes --]

Hi Will,

On Tue, 31 Jul 2018 11:09:18 +0100 Will Deacon <will.deacon@arm.com> wrote:
>
> I've pushed out Laura's fix to the arm64 for-next/core branch, so this
> should be resolved in the next next.

Thanks.  I actually applied Laura's patch to today's linux-next as well.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Will Deacon]

On Mon, Jul 30, 2018 at 05:33:56PM +1000, Stephen Rothwell wrote:
> On Fri, 27 Jul 2018 13:55:22 +0100 Will Deacon <will.deacon@arm.com> wrote:
> > Thanks, Stephen. I managed to reproduce this by merging for-next/kspp from
> > Kees's tree and for-next/core from the arm64 tree. The failure happens when
> > building the EFI stub, so the commit you mention above is almost certainly
> > the culprit.
> > 
> > We build the stub with the following GCC invocation:
> > 
> >  gcc -Wp,-MD,drivers/firmware/efi/libstub/.efi-stub-helper.o.d  -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/4.9/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mcmodel=small -m64 -D__KERNEL__ -O2 -fPIC -fno-strict-aliasing -mno-red-zone -mno-mmx -mno-sse -fshort-wchar -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY -ffreestanding -fno-stack-protector -fplugin-arg-stackleak_plugin-disable   -fno-builtin      -DKBUILD_BASENAME='"efi_stub_helper"' -DKBUILD_MODNAME='"efi_stub_helper"' -c -o drivers/firmware/efi/libstub/.tmp_efi-stub-helper.o drivers/firmware/efi/libstub/ef
 i-stub-helper.c
> > 
> > so given that we're not passing any -fplugin= option anyway (because we
> > override KBUILD_CFLAGS for the stub), I don't understand why we need
> > to the disable option at all.
> > 
> > Laura?
> 
> So today I am just trying reverting that arm64 tree commit.

I've pushed out Laura's fix to the arm64 for-next/core branch, so this
should be resolved in the next next.

Will

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

[Will Deacon]

Hi Laura,

On Mon, Jul 30, 2018 at 07:47:52AM -0700, Laura Abbott wrote:
> On 07/30/2018 12:33 AM, Stephen Rothwell wrote:
> >On Fri, 27 Jul 2018 13:55:22 +0100 Will Deacon <will.deacon@arm.com> wrote:
> >>On Fri, Jul 27, 2018 at 08:55:11PM +1000, Stephen Rothwell wrote:
> >>>Actually, it may have been caused by commit
> >>>
> >>>   0b3e336601b8 ("arm64: Add support for STACKLEAK gcc plugin")
> >>>
> >>>from the arm64 tree.
> >>
> >>Thanks, Stephen. I managed to reproduce this by merging for-next/kspp from
> >>Kees's tree and for-next/core from the arm64 tree. The failure happens when
> >>building the EFI stub, so the commit you mention above is almost certainly
> >>the culprit.
> >>
> >>We build the stub with the following GCC invocation:
> >>
> >>  gcc -Wp,-MD,drivers/firmware/efi/libstub/.efi-stub-helper.o.d  -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/4.9/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mcmodel=small -m64 -D__KERNEL__ -O2 -fPIC -fno-strict-aliasing -mno-red-zone -mno-mmx -mno-sse -fshort-wchar -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY -ffreestanding -fno-stack-protector -fplugin-arg-stackleak_plugin-disable   -fno-builtin      -DKBUILD_BASENAME='"efi_stub_helper"' -DKBUILD_MODNAME='"efi_stub_helper"' -c -o drivers/firmware/efi/libstub/.tmp_efi-stub-helper.o drivers/firmware/efi/libstub/e
 fi-stub-helper.c
> >>
> >>so given that we're not passing any -fplugin= option anyway (because we
> >>override KBUILD_CFLAGS for the stub), I don't understand why we need
> >>to the disable option at all.
> >>
> >>Laura?
> >
> >So today I am just trying reverting that arm64 tree commit.
> >
> 
> It looks like arm and arm64 start from the KBUILD_CFLAGS and
> then filter out vs. x86 which just specifies the CFLAGS individually,
> hence x86 picking up the disable when there's no plugin at all. This
> seems to be the simplest fix unless we want to change arm64 to not
> pick up all the KBUILD_CFLAGS to match x86. That seems like a more
> involved process though. If this is okay, I can send a patch
> that also sticks a comment in there explaining why fixing on arm64
> is necessary.

Indeed, I posted a very similar patch last week!

https://lore.kernel.org/lkml/CAGXu5jJ=0YBYKkQM3=KZRp1o3fT0yGY6-0UDkkit0WenFM3oDg@mail.gmail.com/T/#m1bd3d2de78e33da4d1f496fd82be7cf088ebaa06

If you send a version with a commit message, I'm happy to pick it up.

Cheers,

Will

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

[Laura Abbott]

On 07/30/2018 12:33 AM, Stephen Rothwell wrote:
> Hi all,
> 
> On Fri, 27 Jul 2018 13:55:22 +0100 Will Deacon <will.deacon@arm.com> wrote:
>>
>> On Fri, Jul 27, 2018 at 08:55:11PM +1000, Stephen Rothwell wrote:
>>> On Fri, 27 Jul 2018 19:06:47 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>>>
>>>> On Fri, 27 Jul 2018 19:02:07 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>>>>
>>>>> After merging the kspp tree, today's linux-next build (x86_64
>>>>> allmodconfig) failed like this:
>>>>>
>>>>> cc1: error: plugin stackleak_plugin should be specified before -fplugin-arg-stackleak_plugin-disable in the command line
>>>>>
>>>>> Maybe caused by commit
>>>>>
>>>>>    a8b9eaddb9c0 ("gcc-plugins: Add STACKLEAK plugin for tracking the kernel stack")
>>>>>
>>>>> I have used the kspp tree from next-20180726 for today.
>>>>
>>>> Well, that obviously didn't work since the tree hasn't changed for a
>>>> few days.
>>>>
>>>> I can't see what has interacted to make this happen, so I have dropped
>>>> the kspp tree for today.
>>>
>>> Actually, it may have been caused by commit
>>>
>>>    0b3e336601b8 ("arm64: Add support for STACKLEAK gcc plugin")
>>>
>>> from the arm64 tree.
>>
>> Thanks, Stephen. I managed to reproduce this by merging for-next/kspp from
>> Kees's tree and for-next/core from the arm64 tree. The failure happens when
>> building the EFI stub, so the commit you mention above is almost certainly
>> the culprit.
>>
>> We build the stub with the following GCC invocation:
>>
>>   gcc -Wp,-MD,drivers/firmware/efi/libstub/.efi-stub-helper.o.d  -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/4.9/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mcmodel=small -m64 -D__KERNEL__ -O2 -fPIC -fno-strict-aliasing -mno-red-zone -mno-mmx -mno-sse -fshort-wchar -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY -ffreestanding -fno-stack-protector -fplugin-arg-stackleak_plugin-disable   -fno-builtin      -DKBUILD_BASENAME='"efi_stub_helper"' -DKBUILD_MODNAME='"efi_stub_helper"' -c -o drivers/firmware/efi/libstub/.tmp_efi-stub-helper.o drivers/firmware/efi/libstub/ef
 i-stub-helper.c
>>
>> so given that we're not passing any -fplugin= option anyway (because we
>> override KBUILD_CFLAGS for the stub), I don't understand why we need
>> to the disable option at all.
>>
>> Laura?
> 
> So today I am just trying reverting that arm64 tree commit.
> 

It looks like arm and arm64 start from the KBUILD_CFLAGS and
then filter out vs. x86 which just specifies the CFLAGS individually,
hence x86 picking up the disable when there's no plugin at all. This
seems to be the simplest fix unless we want to change arm64 to not
pick up all the KBUILD_CFLAGS to match x86. That seems like a more
involved process though. If this is okay, I can send a patch
that also sticks a comment in there explaining why fixing on arm64
is necessary.

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 25dd2a14560d..31f376279d5c 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -11,7 +11,8 @@ cflags-$(CONFIG_X86)		+= -m$(BITS) -D__KERNEL__ -O2 \
  				   -fPIC -fno-strict-aliasing -mno-red-zone \
  				   -mno-mmx -mno-sse -fshort-wchar
  
-cflags-$(CONFIG_ARM64)		:= $(subst -pg,,$(KBUILD_CFLAGS)) -fpie
+cflags-$(CONFIG_ARM64)		:= $(subst -pg,,$(KBUILD_CFLAGS)) -fpie \
+				   $(DISABLE_STACKLEAK_PLUGIN)
  cflags-$(CONFIG_ARM)		:= $(subst -pg,,$(KBUILD_CFLAGS)) \
  				   -fno-builtin -fpic -mno-single-pic-base
  
@@ -21,7 +22,6 @@ KBUILD_CFLAGS			:= $(cflags-y) -DDISABLE_BRANCH_PROFILING \
  				   -D__NO_FORTIFY \
  				   $(call cc-option,-ffreestanding) \
  				   $(call cc-option,-fno-stack-protector) \
-				   $(DISABLE_STACKLEAK_PLUGIN)
  
  GCOV_PROFILE			:= n
  KASAN_SANITIZE			:= n

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2644 bytes --]

Hi all,

On Fri, 27 Jul 2018 13:55:22 +0100 Will Deacon <will.deacon@arm.com> wrote:
>
> On Fri, Jul 27, 2018 at 08:55:11PM +1000, Stephen Rothwell wrote:
> > On Fri, 27 Jul 2018 19:06:47 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:  
> > >
> > > On Fri, 27 Jul 2018 19:02:07 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:  
> > > >
> > > > After merging the kspp tree, today's linux-next build (x86_64
> > > > allmodconfig) failed like this:
> > > > 
> > > > cc1: error: plugin stackleak_plugin should be specified before -fplugin-arg-stackleak_plugin-disable in the command line
> > > > 
> > > > Maybe caused by commit
> > > > 
> > > >   a8b9eaddb9c0 ("gcc-plugins: Add STACKLEAK plugin for tracking the kernel stack")
> > > > 
> > > > I have used the kspp tree from next-20180726 for today.    
> > > 
> > > Well, that obviously didn't work since the tree hasn't changed for a
> > > few days.
> > > 
> > > I can't see what has interacted to make this happen, so I have dropped
> > > the kspp tree for today.  
> > 
> > Actually, it may have been caused by commit
> > 
> >   0b3e336601b8 ("arm64: Add support for STACKLEAK gcc plugin")
> > 
> > from the arm64 tree.  
> 
> Thanks, Stephen. I managed to reproduce this by merging for-next/kspp from
> Kees's tree and for-next/core from the arm64 tree. The failure happens when
> building the EFI stub, so the commit you mention above is almost certainly
> the culprit.
> 
> We build the stub with the following GCC invocation:
> 
>  gcc -Wp,-MD,drivers/firmware/efi/libstub/.efi-stub-helper.o.d  -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/4.9/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mcmodel=small -m64 -D__KERNEL__ -O2 -fPIC -fno-strict-aliasing -mno-red-zone -mno-mmx -mno-sse -fshort-wchar -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY -ffreestanding -fno-stack-protector -fplugin-arg-stackleak_plugin-disable   -fno-builtin      -DKBUILD_BASENAME='"efi_stub_helper"' -DKBUILD_MODNAME='"efi_stub_helper"' -c -o drivers/firmware/efi/libstub/.tmp_efi-stub-helper.o drivers/firmware/efi/libstub/efi-stub-helper.c
> 
> so given that we're not passing any -fplugin= option anyway (because we
> override KBUILD_CFLAGS for the stub), I don't understand why we need
> to the disable option at all.
> 
> Laura?

So today I am just trying reverting that arm64 tree commit.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Fri, Jul 27, 2018 at 6:27 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Fri, Jul 27, 2018 at 02:01:06PM +0100, Will Deacon wrote:
>> On Fri, Jul 27, 2018 at 01:55:22PM +0100, Will Deacon wrote:
>> > On Fri, Jul 27, 2018 at 08:55:11PM +1000, Stephen Rothwell wrote:
>> > > On Fri, 27 Jul 2018 19:06:47 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>> > > >
>> > > > On Fri, 27 Jul 2018 19:02:07 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>> > > > >
>> > > > > After merging the kspp tree, today's linux-next build (x86_64
>> > > > > allmodconfig) failed like this:
>> > > > >
>> > > > > cc1: error: plugin stackleak_plugin should be specified before -fplugin-arg-stackleak_plugin-disable in the command line
>> > > > >
>> > > > > Maybe caused by commit
>> > > > >
>> > > > >   a8b9eaddb9c0 ("gcc-plugins: Add STACKLEAK plugin for tracking the kernel stack")
>> > > > >
>> > > > > I have used the kspp tree from next-20180726 for today.
>> > > >
>> > > > Well, that obviously didn't work since the tree hasn't changed for a
>> > > > few days.
>> > > >
>> > > > I can't see what has interacted to make this happen, so I have dropped
>> > > > the kspp tree for today.
>> > >
>> > > Actually, it may have been caused by commit
>> > >
>> > >   0b3e336601b8 ("arm64: Add support for STACKLEAK gcc plugin")
>> > >
>> > > from the arm64 tree.
>> >
>> > Thanks, Stephen. I managed to reproduce this by merging for-next/kspp from
>> > Kees's tree and for-next/core from the arm64 tree. The failure happens when
>> > building the EFI stub, so the commit you mention above is almost certainly
>> > the culprit.
>> >
>> > We build the stub with the following GCC invocation:
>> >
>> >  gcc -Wp,-MD,drivers/firmware/efi/libstub/.efi-stub-helper.o.d  -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/4.9/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mcmodel=small -m64 -D__KERNEL__ -O2 -fPIC -fno-strict-aliasing -mno-red-zone -mno-mmx -mno-sse -fshort-wchar -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY -ffreestanding -fno-stack-protector -fplugin-arg-stackleak_plugin-disable   -fno-builtin      -DKBUILD_BASENAME='"efi_stub_helper"' -DKBUILD_MODNAME='"efi_stub_helper"' -c -o drivers/firmware/efi/libstub/.tmp_efi-stub-helper.o drivers/firmware/efi/libstub/efi-stub-helper.c
>> >
>> > so given that we're not passing any -fplugin= option anyway (because we
>> > override KBUILD_CFLAGS for the stub), I don't understand why we need
>> > to the disable option at all.
>> >
>> > Laura?
>>
>> ... ah, but arm and arm64 inherit the old KBUILD_CFLAGS via the
>> cflags-$(CONFIG_ARM64) and cflags-$(CONFIG_ARM) definitions, so they
>> would be the places where we need to disable the plugin.
>
> i.e. something like the diff below.
>
> Will
>
> --->8
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index 25dd2a14560d..f3700fe08908 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -11,9 +11,11 @@ cflags-$(CONFIG_X86)         += -m$(BITS) -D__KERNEL__ -O2 \
>                                    -fPIC -fno-strict-aliasing -mno-red-zone \
>                                    -mno-mmx -mno-sse -fshort-wchar
>
> -cflags-$(CONFIG_ARM64)         := $(subst -pg,,$(KBUILD_CFLAGS)) -fpie
> +cflags-$(CONFIG_ARM64)         := $(subst -pg,,$(KBUILD_CFLAGS)) -fpie \
> +                                  $(DISABLE_STACKLEAK_PLUGIN)
>  cflags-$(CONFIG_ARM)           := $(subst -pg,,$(KBUILD_CFLAGS)) \
> -                                  -fno-builtin -fpic -mno-single-pic-base
> +                                  -fno-builtin -fpic -mno-single-pic-base \
> +                                  $(DISABLE_STACKLEAK_PLUGIN)
>
>  cflags-$(CONFIG_EFI_ARMSTUB)   += -I$(srctree)/scripts/dtc/libfdt
>
> @@ -21,7 +23,6 @@ KBUILD_CFLAGS                 := $(cflags-y) -DDISABLE_BRANCH_PROFILING \
>                                    -D__NO_FORTIFY \
>                                    $(call cc-option,-ffreestanding) \
>                                    $(call cc-option,-fno-stack-protector) \
> -                                  $(DISABLE_STACKLEAK_PLUGIN)
>
>  GCOV_PROFILE                   := n
>  KASAN_SANITIZE                 := n

Ah! Thanks for tracking this down!

-Kees

-- 
Kees Cook
Pixel Security

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

[Will Deacon]

On Fri, Jul 27, 2018 at 02:01:06PM +0100, Will Deacon wrote:
> On Fri, Jul 27, 2018 at 01:55:22PM +0100, Will Deacon wrote:
> > On Fri, Jul 27, 2018 at 08:55:11PM +1000, Stephen Rothwell wrote:
> > > On Fri, 27 Jul 2018 19:06:47 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > >
> > > > On Fri, 27 Jul 2018 19:02:07 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > > >
> > > > > After merging the kspp tree, today's linux-next build (x86_64
> > > > > allmodconfig) failed like this:
> > > > > 
> > > > > cc1: error: plugin stackleak_plugin should be specified before -fplugin-arg-stackleak_plugin-disable in the command line
> > > > > 
> > > > > Maybe caused by commit
> > > > > 
> > > > >   a8b9eaddb9c0 ("gcc-plugins: Add STACKLEAK plugin for tracking the kernel stack")
> > > > > 
> > > > > I have used the kspp tree from next-20180726 for today.  
> > > > 
> > > > Well, that obviously didn't work since the tree hasn't changed for a
> > > > few days.
> > > > 
> > > > I can't see what has interacted to make this happen, so I have dropped
> > > > the kspp tree for today.
> > > 
> > > Actually, it may have been caused by commit
> > > 
> > >   0b3e336601b8 ("arm64: Add support for STACKLEAK gcc plugin")
> > > 
> > > from the arm64 tree.
> > 
> > Thanks, Stephen. I managed to reproduce this by merging for-next/kspp from
> > Kees's tree and for-next/core from the arm64 tree. The failure happens when
> > building the EFI stub, so the commit you mention above is almost certainly
> > the culprit.
> > 
> > We build the stub with the following GCC invocation:
> > 
> >  gcc -Wp,-MD,drivers/firmware/efi/libstub/.efi-stub-helper.o.d  -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/4.9/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mcmodel=small -m64 -D__KERNEL__ -O2 -fPIC -fno-strict-aliasing -mno-red-zone -mno-mmx -mno-sse -fshort-wchar -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY -ffreestanding -fno-stack-protector -fplugin-arg-stackleak_plugin-disable   -fno-builtin      -DKBUILD_BASENAME='"efi_stub_helper"' -DKBUILD_MODNAME='"efi_stub_helper"' -c -o drivers/firmware/efi/libstub/.tmp_efi-stub-helper.o drivers/firmware/efi/libstub/ef
 i-stub-helper.c
> > 
> > so given that we're not passing any -fplugin= option anyway (because we
> > override KBUILD_CFLAGS for the stub), I don't understand why we need
> > to the disable option at all.
> > 
> > Laura?
> 
> ... ah, but arm and arm64 inherit the old KBUILD_CFLAGS via the
> cflags-$(CONFIG_ARM64) and cflags-$(CONFIG_ARM) definitions, so they
> would be the places where we need to disable the plugin.

i.e. something like the diff below.

Will

--->8

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 25dd2a14560d..f3700fe08908 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -11,9 +11,11 @@ cflags-$(CONFIG_X86)         += -m$(BITS) -D__KERNEL__ -O2 \
                                   -fPIC -fno-strict-aliasing -mno-red-zone \
                                   -mno-mmx -mno-sse -fshort-wchar
 
-cflags-$(CONFIG_ARM64)         := $(subst -pg,,$(KBUILD_CFLAGS)) -fpie
+cflags-$(CONFIG_ARM64)         := $(subst -pg,,$(KBUILD_CFLAGS)) -fpie \
+                                  $(DISABLE_STACKLEAK_PLUGIN)
 cflags-$(CONFIG_ARM)           := $(subst -pg,,$(KBUILD_CFLAGS)) \
-                                  -fno-builtin -fpic -mno-single-pic-base
+                                  -fno-builtin -fpic -mno-single-pic-base \
+                                  $(DISABLE_STACKLEAK_PLUGIN)
 
 cflags-$(CONFIG_EFI_ARMSTUB)   += -I$(srctree)/scripts/dtc/libfdt
 
@@ -21,7 +23,6 @@ KBUILD_CFLAGS                 := $(cflags-y) -DDISABLE_BRANCH_PROFILING \
                                   -D__NO_FORTIFY \
                                   $(call cc-option,-ffreestanding) \
                                   $(call cc-option,-fno-stack-protector) \
-                                  $(DISABLE_STACKLEAK_PLUGIN)
 
 GCOV_PROFILE                   := n
 KASAN_SANITIZE                 := n

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

[Will Deacon]

On Fri, Jul 27, 2018 at 01:55:22PM +0100, Will Deacon wrote:
> On Fri, Jul 27, 2018 at 08:55:11PM +1000, Stephen Rothwell wrote:
> > On Fri, 27 Jul 2018 19:06:47 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > >
> > > On Fri, 27 Jul 2018 19:02:07 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > >
> > > > After merging the kspp tree, today's linux-next build (x86_64
> > > > allmodconfig) failed like this:
> > > > 
> > > > cc1: error: plugin stackleak_plugin should be specified before -fplugin-arg-stackleak_plugin-disable in the command line
> > > > 
> > > > Maybe caused by commit
> > > > 
> > > >   a8b9eaddb9c0 ("gcc-plugins: Add STACKLEAK plugin for tracking the kernel stack")
> > > > 
> > > > I have used the kspp tree from next-20180726 for today.  
> > > 
> > > Well, that obviously didn't work since the tree hasn't changed for a
> > > few days.
> > > 
> > > I can't see what has interacted to make this happen, so I have dropped
> > > the kspp tree for today.
> > 
> > Actually, it may have been caused by commit
> > 
> >   0b3e336601b8 ("arm64: Add support for STACKLEAK gcc plugin")
> > 
> > from the arm64 tree.
> 
> Thanks, Stephen. I managed to reproduce this by merging for-next/kspp from
> Kees's tree and for-next/core from the arm64 tree. The failure happens when
> building the EFI stub, so the commit you mention above is almost certainly
> the culprit.
> 
> We build the stub with the following GCC invocation:
> 
>  gcc -Wp,-MD,drivers/firmware/efi/libstub/.efi-stub-helper.o.d  -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/4.9/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mcmodel=small -m64 -D__KERNEL__ -O2 -fPIC -fno-strict-aliasing -mno-red-zone -mno-mmx -mno-sse -fshort-wchar -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY -ffreestanding -fno-stack-protector -fplugin-arg-stackleak_plugin-disable   -fno-builtin      -DKBUILD_BASENAME='"efi_stub_helper"' -DKBUILD_MODNAME='"efi_stub_helper"' -c -o drivers/firmware/efi/libstub/.tmp_efi-stub-helper.o drivers/firmware/efi/libstub/efi-
 stub-helper.c
> 
> so given that we're not passing any -fplugin= option anyway (because we
> override KBUILD_CFLAGS for the stub), I don't understand why we need
> to the disable option at all.
> 
> Laura?

... ah, but arm and arm64 inherit the old KBUILD_CFLAGS via the
cflags-$(CONFIG_ARM64) and cflags-$(CONFIG_ARM) definitions, so they
would be the places where we need to disable the plugin.

Will

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

[Will Deacon]

On Fri, Jul 27, 2018 at 08:55:11PM +1000, Stephen Rothwell wrote:
> On Fri, 27 Jul 2018 19:06:47 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > On Fri, 27 Jul 2018 19:02:07 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > >
> > > After merging the kspp tree, today's linux-next build (x86_64
> > > allmodconfig) failed like this:
> > > 
> > > cc1: error: plugin stackleak_plugin should be specified before -fplugin-arg-stackleak_plugin-disable in the command line
> > > 
> > > Maybe caused by commit
> > > 
> > >   a8b9eaddb9c0 ("gcc-plugins: Add STACKLEAK plugin for tracking the kernel stack")
> > > 
> > > I have used the kspp tree from next-20180726 for today.  
> > 
> > Well, that obviously didn't work since the tree hasn't changed for a
> > few days.
> > 
> > I can't see what has interacted to make this happen, so I have dropped
> > the kspp tree for today.
> 
> Actually, it may have been caused by commit
> 
>   0b3e336601b8 ("arm64: Add support for STACKLEAK gcc plugin")
> 
> from the arm64 tree.

Thanks, Stephen. I managed to reproduce this by merging for-next/kspp from
Kees's tree and for-next/core from the arm64 tree. The failure happens when
building the EFI stub, so the commit you mention above is almost certainly
the culprit.

We build the stub with the following GCC invocation:

 gcc -Wp,-MD,drivers/firmware/efi/libstub/.efi-stub-helper.o.d  -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/4.9/include -I./arch/x86/include -I./arch/x86/include/generated  -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mcmodel=small -m64 -D__KERNEL__ -O2 -fPIC -fno-strict-aliasing -mno-red-zone -mno-mmx -mno-sse -fshort-wchar -DDISABLE_BRANCH_PROFILING -D__NO_FORTIFY -ffreestanding -fno-stack-protector -fplugin-arg-stackleak_plugin-disable   -fno-builtin      -DKBUILD_BASENAME='"efi_stub_helper"' -DKBUILD_MODNAME='"efi_stub_helper"' -c -o drivers/firmware/efi/libstub/.tmp_efi-stub-helper.o drivers/firmware/efi/libstub/efi-st
 ub-helper.c

so given that we're not passing any -fplugin= option anyway (because we
override KBUILD_CFLAGS for the stub), I don't understand why we need
to the disable option at all.

Laura?

Will

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

Hi all,

On Fri, 27 Jul 2018 19:06:47 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Fri, 27 Jul 2018 19:02:07 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > After merging the kspp tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> > 
> > cc1: error: plugin stackleak_plugin should be specified before -fplugin-arg-stackleak_plugin-disable in the command line
> > 
> > Maybe caused by commit
> > 
> >   a8b9eaddb9c0 ("gcc-plugins: Add STACKLEAK plugin for tracking the kernel stack")
> > 
> > I have used the kspp tree from next-20180726 for today.  
> 
> Well, that obviously didn't work since the tree hasn't changed for a
> few days.
> 
> I can't see what has interacted to make this happen, so I have dropped
> the kspp tree for today.

Actually, it may have been caused by commit

  0b3e336601b8 ("arm64: Add support for STACKLEAK gcc plugin")

from the arm64 tree.
-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Hi all,

On Fri, 27 Jul 2018 19:02:07 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> After merging the kspp tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> cc1: error: plugin stackleak_plugin should be specified before -fplugin-arg-stackleak_plugin-disable in the command line
> 
> Maybe caused by commit
> 
>   a8b9eaddb9c0 ("gcc-plugins: Add STACKLEAK plugin for tracking the kernel stack")
> 
> I have used the kspp tree from next-20180726 for today.

Well, that obviously didn't work since the tree hasn't changed for a
few days.

I can't see what has interacted to make this happen, so I have dropped
the kspp tree for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

Hi Kees,

After merging the kspp tree, today's linux-next build (x86_64
allmodconfig) failed like this:

cc1: error: plugin stackleak_plugin should be specified before -fplugin-arg-stackleak_plugin-disable in the command line

Maybe caused by commit

  a8b9eaddb9c0 ("gcc-plugins: Add STACKLEAK plugin for tracking the kernel stack")

I have used the kspp tree from next-20180726 for today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Kees Cook]

On Wed, Nov 8, 2017 at 4:18 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> Agreed.  I guess we'll see you for round X when you get to general
> kmalloc annotating. :)

That should be "fun". :)

-Kees

-- 
Kees Cook
Pixel Security

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

[Darrick J. Wong]

On Wed, Nov 08, 2017 at 03:43:47PM -0800, Kees Cook wrote:
> On Tue, Nov 7, 2017 at 9:23 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > Hi Kees,
> >
> > After merging the kspp tree, today's linux-next build (powerpc
> > ppc64_defconfig) failed like this:
> >
> > In file included from include/linux/compiler_types.h:58:0,
> >                  from include/uapi/linux/stddef.h:2,
> >                  from include/linux/stddef.h:5,
> >                  from include/uapi/linux/posix_types.h:5,
> >                  from include/uapi/linux/types.h:14,
> >                  from include/linux/types.h:6,
> >                  from fs/xfs/xfs_linux.h:21,
> >                  from fs/xfs/xfs.h:35,
> >                  from fs/xfs/xfs_super.c:19:
> > fs/xfs/xfs_super.c: In function 'xfs_init_zones':
> > include/linux/compiler-gcc.h:166:2: error: 'xfs_ifork_t {aka struct xfs_ifork}' has no member named 'if_u2'
> >   __builtin_offsetof(a, b)
> >   ^
> > include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
> >  #define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
> >                                 ^
> > fs/xfs/xfs_super.c:1862:4: note: in expansion of macro 'offsetof'
> >     offsetof(xfs_inode_t, i_df.if_u2.if_inline_data),
> >     ^
> > In file included from include/uapi/linux/posix_types.h:5:0,
> >                  from include/uapi/linux/types.h:14,
> >                  from include/linux/types.h:6,
> >                  from fs/xfs/xfs_linux.h:21,
> >                  from fs/xfs/xfs.h:35,
> >                  from fs/xfs/xfs_super.c:19:
> > fs/xfs/xfs_super.c:1863:34: error: 'xfs_ifork_t {aka struct xfs_ifork}' has no member named 'if_u2'
> >     sizeof_field(xfs_inode_t, i_df.if_u2.if_inline_data),
> >                                   ^
> > include/linux/stddef.h:22:66: note: in definition of macro 'sizeof_field'
> >  #define sizeof_field(structure, field) sizeof((((structure *)0)->field))
> >                                                                   ^
> >
> > Caused by commit
> >
> >   1d48144b9688 ("xfs: Define usercopy region in xfs_inode slab cache")
> >
> > interacting with commit
> >
> >   43518812d297 ("xfs: remove support for inlining data/extents into the inode fork")
> >
> > from the  tree.
> >
> > I just reverted the kspp tree commit as it seems like it is no longer
> > needed.
> 
> Yup, that looks like the correct fix. Thanks!

Agreed.  I guess we'll see you for round X when you get to general
kmalloc annotating. :)

--D

> -Kees
> 
> -- 
> Kees Cook
> Pixel Security
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Kees Cook]

On Tue, Nov 7, 2017 at 9:23 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Kees,
>
> After merging the kspp tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
>
> In file included from include/linux/compiler_types.h:58:0,
>                  from include/uapi/linux/stddef.h:2,
>                  from include/linux/stddef.h:5,
>                  from include/uapi/linux/posix_types.h:5,
>                  from include/uapi/linux/types.h:14,
>                  from include/linux/types.h:6,
>                  from fs/xfs/xfs_linux.h:21,
>                  from fs/xfs/xfs.h:35,
>                  from fs/xfs/xfs_super.c:19:
> fs/xfs/xfs_super.c: In function 'xfs_init_zones':
> include/linux/compiler-gcc.h:166:2: error: 'xfs_ifork_t {aka struct xfs_ifork}' has no member named 'if_u2'
>   __builtin_offsetof(a, b)
>   ^
> include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
>  #define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
>                                 ^
> fs/xfs/xfs_super.c:1862:4: note: in expansion of macro 'offsetof'
>     offsetof(xfs_inode_t, i_df.if_u2.if_inline_data),
>     ^
> In file included from include/uapi/linux/posix_types.h:5:0,
>                  from include/uapi/linux/types.h:14,
>                  from include/linux/types.h:6,
>                  from fs/xfs/xfs_linux.h:21,
>                  from fs/xfs/xfs.h:35,
>                  from fs/xfs/xfs_super.c:19:
> fs/xfs/xfs_super.c:1863:34: error: 'xfs_ifork_t {aka struct xfs_ifork}' has no member named 'if_u2'
>     sizeof_field(xfs_inode_t, i_df.if_u2.if_inline_data),
>                                   ^
> include/linux/stddef.h:22:66: note: in definition of macro 'sizeof_field'
>  #define sizeof_field(structure, field) sizeof((((structure *)0)->field))
>                                                                   ^
>
> Caused by commit
>
>   1d48144b9688 ("xfs: Define usercopy region in xfs_inode slab cache")
>
> interacting with commit
>
>   43518812d297 ("xfs: remove support for inlining data/extents into the inode fork")
>
> from the  tree.
>
> I just reverted the kspp tree commit as it seems like it is no longer
> needed.

Yup, that looks like the correct fix. Thanks!

-Kees

-- 
Kees Cook
Pixel Security

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

[Stephen Rothwell]

Hi Kees,

After merging the kspp tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

In file included from include/linux/compiler_types.h:58:0,
                 from include/uapi/linux/stddef.h:2,
                 from include/linux/stddef.h:5,
                 from include/uapi/linux/posix_types.h:5,
                 from include/uapi/linux/types.h:14,
                 from include/linux/types.h:6,
                 from fs/xfs/xfs_linux.h:21,
                 from fs/xfs/xfs.h:35,
                 from fs/xfs/xfs_super.c:19:
fs/xfs/xfs_super.c: In function 'xfs_init_zones':
include/linux/compiler-gcc.h:166:2: error: 'xfs_ifork_t {aka struct xfs_ifork}' has no member named 'if_u2'
  __builtin_offsetof(a, b)
  ^
include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
 #define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
                                ^
fs/xfs/xfs_super.c:1862:4: note: in expansion of macro 'offsetof'
    offsetof(xfs_inode_t, i_df.if_u2.if_inline_data),
    ^
In file included from include/uapi/linux/posix_types.h:5:0,
                 from include/uapi/linux/types.h:14,
                 from include/linux/types.h:6,
                 from fs/xfs/xfs_linux.h:21,
                 from fs/xfs/xfs.h:35,
                 from fs/xfs/xfs_super.c:19:
fs/xfs/xfs_super.c:1863:34: error: 'xfs_ifork_t {aka struct xfs_ifork}' has no member named 'if_u2'
    sizeof_field(xfs_inode_t, i_df.if_u2.if_inline_data),
                                  ^
include/linux/stddef.h:22:66: note: in definition of macro 'sizeof_field'
 #define sizeof_field(structure, field) sizeof((((structure *)0)->field))
                                                                  ^

Caused by commit

  1d48144b9688 ("xfs: Define usercopy region in xfs_inode slab cache")

interacting with commit

  43518812d297 ("xfs: remove support for inlining data/extents into the inode fork")

from the  tree.

I just reverted the kspp tree commit as it seems like it is no longer
needed.

-- 
Cheers,
Stephen Rothwell

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

[James Morris]

On Tue, 27 Jun 2017, Kees Cook wrote:

> On Mon, Jun 26, 2017 at 8:33 PM, James Morris <jmorris@namei.org> wrote:
> > On Mon, 26 Jun 2017, Kees Cook wrote:
> >
> >> >> Fixes: 8014370f1257 ("apparmor: move path_link mediation to using labels")
> >> >> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> >> > Acked-by: John Johansen <john.johansen@canonical.com>
> >>
> >> Hi James,
> >>
> >> Just a ping; this needs to get into -next to avoid build errors.
> >
> > Surely Linus will resolve this when he pulls the trees in?
> 
> It's not a merge glitch, it's a refactoring glitch. John's commit in
> security-next ("apparmor: move path_link mediation to using labels")
> undid an earlier commit 8486adf0d755 ("apparmor: use designated
> initializers") from v4.11. This patch is needed for security-next.
> 

Thanks.

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next


-- 
James Morris
<jmorris@namei.org>

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

[Kees Cook]

On Mon, Jun 26, 2017 at 8:33 PM, James Morris <jmorris@namei.org> wrote:
> On Mon, 26 Jun 2017, Kees Cook wrote:
>
>> >> Fixes: 8014370f1257 ("apparmor: move path_link mediation to using labels")
>> >> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> > Acked-by: John Johansen <john.johansen@canonical.com>
>>
>> Hi James,
>>
>> Just a ping; this needs to get into -next to avoid build errors.
>
> Surely Linus will resolve this when he pulls the trees in?

It's not a merge glitch, it's a refactoring glitch. John's commit in
security-next ("apparmor: move path_link mediation to using labels")
undid an earlier commit 8486adf0d755 ("apparmor: use designated
initializers") from v4.11. This patch is needed for security-next.

-Kees

-- 
Kees Cook
Pixel Security

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

[James Morris]

On Mon, 26 Jun 2017, Kees Cook wrote:

> >> Fixes: 8014370f1257 ("apparmor: move path_link mediation to using labels")
> >> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Acked-by: John Johansen <john.johansen@canonical.com>
> 
> Hi James,
> 
> Just a ping; this needs to get into -next to avoid build errors.

Surely Linus will resolve this when he pulls the trees in?


-- 
James Morris
<jmorris@namei.org>

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

[Kees Cook]

On Mon, Jun 19, 2017 at 10:39 PM, John Johansen
<john.johansen@canonical.com> wrote:
> On 06/19/2017 09:56 PM, Stephen Rothwell wrote:
>> Hi all,
>>
>> After merging the kspp tree, today's linux-next build (x86_64
>> allmodconfig) failed like this:
>>
>> security/apparmor/file.c: In function 'aa_path_link':
>> security/apparmor/file.c:475:23: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>>   struct path link = { new_dir->mnt, new_dentry };
>>                        ^
>> security/apparmor/file.c:475:23: note: (near initialization for 'link')
>> security/apparmor/file.c:475:37: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>>   struct path link = { new_dir->mnt, new_dentry };
>>                                      ^
>> security/apparmor/file.c:475:37: note: (near initialization for 'link')
>> security/apparmor/file.c:476:25: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>>   struct path target = { new_dir->mnt, old_dentry };
>>                          ^
>> security/apparmor/file.c:476:25: note: (near initialization for 'target')
>> security/apparmor/file.c:476:39: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>>   struct path target = { new_dir->mnt, old_dentry };
>>                                        ^
>> security/apparmor/file.c:476:39: note: (near initialization for 'target')
>>
>> Caused by commit
>>
>>   1a12979f61e4 ("randstruct: Mark various structs for randomization")
>>
>> interacting with commit
>>
>>   8014370f1257 ("apparmor: move path_link mediation to using labels")
>>
>> from the security tree.
>>
>> I added the following merge fix patch for today:
>>
>> From: Stephen Rothwell <sfr@canb.auug.org.au>
>> Date: Tue, 20 Jun 2017 14:50:36 +1000
>> Subject: [PATCH] apparmor: put back designators in struct initialisers
>>
>> Fixes: 8014370f1257 ("apparmor: move path_link mediation to using labels")
>> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Acked-by: John Johansen <john.johansen@canonical.com>

Hi James,

Just a ping; this needs to get into -next to avoid build errors.
Please consider it also:

Acked-by: Kees Cook <keescook@chromium.org>

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

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

[John Johansen]

On 06/19/2017 10:39 PM, Kees Cook wrote:
> On Mon, Jun 19, 2017 at 9:56 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>> Hi all,
>>
>> After merging the kspp tree, today's linux-next build (x86_64
>> allmodconfig) failed like this:
>>
>> security/apparmor/file.c: In function 'aa_path_link':
>> security/apparmor/file.c:475:23: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>>   struct path link = { new_dir->mnt, new_dentry };
>>                        ^
>> security/apparmor/file.c:475:23: note: (near initialization for 'link')
>> security/apparmor/file.c:475:37: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>>   struct path link = { new_dir->mnt, new_dentry };
>>                                      ^
>> security/apparmor/file.c:475:37: note: (near initialization for 'link')
>> security/apparmor/file.c:476:25: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>>   struct path target = { new_dir->mnt, old_dentry };
>>                          ^
>> security/apparmor/file.c:476:25: note: (near initialization for 'target')
>> security/apparmor/file.c:476:39: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>>   struct path target = { new_dir->mnt, old_dentry };
>>                                        ^
>> security/apparmor/file.c:476:39: note: (near initialization for 'target')
>>
>> Caused by commit
>>
>>   1a12979f61e4 ("randstruct: Mark various structs for randomization")
>>
>> interacting with commit
>>
>>   8014370f1257 ("apparmor: move path_link mediation to using labels")
>>
>> from the security tree.
>>
>> I added the following merge fix patch for today:
>>
>> From: Stephen Rothwell <sfr@canb.auug.org.au>
>> Date: Tue, 20 Jun 2017 14:50:36 +1000
>> Subject: [PATCH] apparmor: put back designators in struct initialisers
>>
>> Fixes: 8014370f1257 ("apparmor: move path_link mediation to using labels")
>> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> ---
>>  security/apparmor/file.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/security/apparmor/file.c b/security/apparmor/file.c
>> index b6e8e5b11e05..3382518b87fa 100644
>> --- a/security/apparmor/file.c
>> +++ b/security/apparmor/file.c
>> @@ -472,8 +472,8 @@ static int profile_path_link(struct aa_profile *profile,
>>  int aa_path_link(struct aa_label *label, struct dentry *old_dentry,
>>                  const struct path *new_dir, struct dentry *new_dentry)
>>  {
>> -       struct path link = { new_dir->mnt, new_dentry };
>> -       struct path target = { new_dir->mnt, old_dentry };
>> +       struct path link = { .mnt = new_dir->mnt, .dentry = new_dentry };
>> +       struct path target = { .mnt = new_dir->mnt, .dentry = old_dentry };
>>         struct path_cond cond = {
>>                 d_backing_inode(old_dentry)->i_uid,
>>                 d_backing_inode(old_dentry)->i_mode
>> --
>> 2.11.0
> 
> Thanks for the fix! That looks correct to me. It seems the refactoring
> in 8014370f1257 ("apparmor: move path_link mediation to using labels")
> didn't take 8486adf0d755 ("apparmor: use designated initializers")
> into account. John, if this looks okay, can you Ack it for James to
> carry in security-next?
> 
yep, already done. Sorry I missed that one :(

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

[John Johansen]

On 06/19/2017 09:56 PM, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kspp tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> security/apparmor/file.c: In function 'aa_path_link':
> security/apparmor/file.c:475:23: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>   struct path link = { new_dir->mnt, new_dentry };
>                        ^
> security/apparmor/file.c:475:23: note: (near initialization for 'link')
> security/apparmor/file.c:475:37: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>   struct path link = { new_dir->mnt, new_dentry };
>                                      ^
> security/apparmor/file.c:475:37: note: (near initialization for 'link')
> security/apparmor/file.c:476:25: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>   struct path target = { new_dir->mnt, old_dentry };
>                          ^
> security/apparmor/file.c:476:25: note: (near initialization for 'target')
> security/apparmor/file.c:476:39: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>   struct path target = { new_dir->mnt, old_dentry };
>                                        ^
> security/apparmor/file.c:476:39: note: (near initialization for 'target')
> 
> Caused by commit
> 
>   1a12979f61e4 ("randstruct: Mark various structs for randomization")
> 
> interacting with commit
> 
>   8014370f1257 ("apparmor: move path_link mediation to using labels")
> 
> from the security tree.
> 
> I added the following merge fix patch for today:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 20 Jun 2017 14:50:36 +1000
> Subject: [PATCH] apparmor: put back designators in struct initialisers
> 
> Fixes: 8014370f1257 ("apparmor: move path_link mediation to using labels")
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Acked-by: John Johansen <john.johansen@canonical.com>

> ---
>  security/apparmor/file.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/security/apparmor/file.c b/security/apparmor/file.c
> index b6e8e5b11e05..3382518b87fa 100644
> --- a/security/apparmor/file.c
> +++ b/security/apparmor/file.c
> @@ -472,8 +472,8 @@ static int profile_path_link(struct aa_profile *profile,
>  int aa_path_link(struct aa_label *label, struct dentry *old_dentry,
>  		 const struct path *new_dir, struct dentry *new_dentry)
>  {
> -	struct path link = { new_dir->mnt, new_dentry };
> -	struct path target = { new_dir->mnt, old_dentry };
> +	struct path link = { .mnt = new_dir->mnt, .dentry = new_dentry };
> +	struct path target = { .mnt = new_dir->mnt, .dentry = old_dentry };
>  	struct path_cond cond = {
>  		d_backing_inode(old_dentry)->i_uid,
>  		d_backing_inode(old_dentry)->i_mode
> 

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

[Kees Cook]

On Mon, Jun 19, 2017 at 9:56 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi all,
>
> After merging the kspp tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> security/apparmor/file.c: In function 'aa_path_link':
> security/apparmor/file.c:475:23: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>   struct path link = { new_dir->mnt, new_dentry };
>                        ^
> security/apparmor/file.c:475:23: note: (near initialization for 'link')
> security/apparmor/file.c:475:37: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>   struct path link = { new_dir->mnt, new_dentry };
>                                      ^
> security/apparmor/file.c:475:37: note: (near initialization for 'link')
> security/apparmor/file.c:476:25: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>   struct path target = { new_dir->mnt, old_dentry };
>                          ^
> security/apparmor/file.c:476:25: note: (near initialization for 'target')
> security/apparmor/file.c:476:39: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
>   struct path target = { new_dir->mnt, old_dentry };
>                                        ^
> security/apparmor/file.c:476:39: note: (near initialization for 'target')
>
> Caused by commit
>
>   1a12979f61e4 ("randstruct: Mark various structs for randomization")
>
> interacting with commit
>
>   8014370f1257 ("apparmor: move path_link mediation to using labels")
>
> from the security tree.
>
> I added the following merge fix patch for today:
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 20 Jun 2017 14:50:36 +1000
> Subject: [PATCH] apparmor: put back designators in struct initialisers
>
> Fixes: 8014370f1257 ("apparmor: move path_link mediation to using labels")
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  security/apparmor/file.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/apparmor/file.c b/security/apparmor/file.c
> index b6e8e5b11e05..3382518b87fa 100644
> --- a/security/apparmor/file.c
> +++ b/security/apparmor/file.c
> @@ -472,8 +472,8 @@ static int profile_path_link(struct aa_profile *profile,
>  int aa_path_link(struct aa_label *label, struct dentry *old_dentry,
>                  const struct path *new_dir, struct dentry *new_dentry)
>  {
> -       struct path link = { new_dir->mnt, new_dentry };
> -       struct path target = { new_dir->mnt, old_dentry };
> +       struct path link = { .mnt = new_dir->mnt, .dentry = new_dentry };
> +       struct path target = { .mnt = new_dir->mnt, .dentry = old_dentry };
>         struct path_cond cond = {
>                 d_backing_inode(old_dentry)->i_uid,
>                 d_backing_inode(old_dentry)->i_mode
> --
> 2.11.0

Thanks for the fix! That looks correct to me. It seems the refactoring
in 8014370f1257 ("apparmor: move path_link mediation to using labels")
didn't take 8486adf0d755 ("apparmor: use designated initializers")
into account. John, if this looks okay, can you Ack it for James to
carry in security-next?

-Kees

-- 
Kees Cook
Pixel Security

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

[Stephen Rothwell]

Hi all,

After merging the kspp tree, today's linux-next build (x86_64
allmodconfig) failed like this:

security/apparmor/file.c: In function 'aa_path_link':
security/apparmor/file.c:475:23: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
  struct path link = { new_dir->mnt, new_dentry };
                       ^
security/apparmor/file.c:475:23: note: (near initialization for 'link')
security/apparmor/file.c:475:37: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
  struct path link = { new_dir->mnt, new_dentry };
                                     ^
security/apparmor/file.c:475:37: note: (near initialization for 'link')
security/apparmor/file.c:476:25: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
  struct path target = { new_dir->mnt, old_dentry };
                         ^
security/apparmor/file.c:476:25: note: (near initialization for 'target')
security/apparmor/file.c:476:39: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
  struct path target = { new_dir->mnt, old_dentry };
                                       ^
security/apparmor/file.c:476:39: note: (near initialization for 'target')

Caused by commit

  1a12979f61e4 ("randstruct: Mark various structs for randomization")

interacting with commit

  8014370f1257 ("apparmor: move path_link mediation to using labels")

from the security tree.

I added the following merge fix patch for today:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 20 Jun 2017 14:50:36 +1000
Subject: [PATCH] apparmor: put back designators in struct initialisers

Fixes: 8014370f1257 ("apparmor: move path_link mediation to using labels")
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 security/apparmor/file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index b6e8e5b11e05..3382518b87fa 100644
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -472,8 +472,8 @@ static int profile_path_link(struct aa_profile *profile,
 int aa_path_link(struct aa_label *label, struct dentry *old_dentry,
 		 const struct path *new_dir, struct dentry *new_dentry)
 {
-	struct path link = { new_dir->mnt, new_dentry };
-	struct path target = { new_dir->mnt, old_dentry };
+	struct path link = { .mnt = new_dir->mnt, .dentry = new_dentry };
+	struct path target = { .mnt = new_dir->mnt, .dentry = old_dentry };
 	struct path_cond cond = {
 		d_backing_inode(old_dentry)->i_uid,
 		d_backing_inode(old_dentry)->i_mode
-- 
2.11.0

-- 
Cheers,
Stephen Rothwell