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

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

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

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

[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

[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

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

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

[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

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

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

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

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

[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-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?

Will

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

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

[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

[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/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.
> 

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

[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/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.
> >
> 
> 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

[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/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.

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

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

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

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

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

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

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

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

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

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

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

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

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