linux-kernel - Re: remaining flexible-array conversions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 Apr 2020 09:15:53 -0300
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Nathan Chancellor <natechancellor@...il.com>
Cc:     "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-rdma@...r.kernel.org, clang-built-linux@...glegroups.com
Subject: Re: remaining flexible-array conversions

On Thu, Apr 23, 2020 at 08:47:04PM -0700, Nathan Chancellor wrote:
> Hi Gustavo,
> 
> On Wed, Apr 22, 2020 at 01:26:02PM -0500, Gustavo A. R. Silva wrote:
> > Hi Linus,
> > 
> > Just wanted to ask you if you would agree on pulling the remaining
> > flexible-array conversions all at once, after they bake for a couple
> > of weeks in linux-next[1]
> > 
> > This is not a disruptive change and there are no code generation
> > differences. So, I think it would make better use of everyone's time
> > if you pull this treewide patch[2] from my tree (after sending you a
> > proper pull-request, of course) sometime in the next couple of weeks.
> > 
> > Notice that the treewide patch I mention here has been successfully
> > built (on top of v5.7-rc1) for multiple architectures (arm, arm64,
> > sparc, powerpc, ia64, s390, i386, nios2, c6x, xtensa, openrisc, mips,
> > parisc, x86_64, riscv, sh, sparc64) and 82 different configurations
> > with the help of the 0-day CI guys[3].
> > 
> > What do you think?
> > 
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=d496496793ff69c4a6b1262a0001eb5cd0a56544
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/commit/?h=for-next/kspp&id=d783301058f3d3605f9ad34f0192692ef572d663
> > [3] https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/kernel-ci/kspp-fam0-20200420.md
> > 
> > Thanks
> 
> That patch in -next appears to introduce some warnings with clang when
> CONFIG_UAPI_HEADER_TEST is enabled (allyesconfig/allmodconfig exposed it
> for us with KernelCI [1]):

Indeed, I've tried these conversions before and run into problems like
this, and more. Particularly in userspace these structs also get
embedded in other structs and the warnings explode.

Please drop changes to ib_user_verbs.h from your series

> ./usr/include/rdma/ib_user_verbs.h:436:34: warning: field 'base' with
> variable sized type 'struct ib_uverbs_create_cq_resp' not at the end of
> a struct or class is a GNU extension
> [-Wgnu-variable-sized-type-not-at-end]
>         struct ib_uverbs_create_cq_resp base;
>                                         ^
> ./usr/include/rdma/ib_user_verbs.h:647:34: warning: field 'base' with
> variable sized type 'struct ib_uverbs_create_qp_resp' not at the end of
> a struct or class is a GNU extension
> [-Wgnu-variable-sized-type-not-at-end]
>         struct ib_uverbs_create_qp_resp base;
>                                         ^
> ./usr/include/rdma/ib_user_verbs.h:743:29: warning: field 'base' with
> variable sized type 'struct ib_uverbs_modify_qp' not at the end of a
> struct or class is a GNU extension
> [-Wgnu-variable-sized-type-not-at-end]
>         struct ib_uverbs_modify_qp base;
>                                    ^
> 3 warnings generated.
> 
> I presume this is part of the point of the conversion since you mention
> a compiler warning when the flexible member is not at the end of a
> struct. How should they be fixed? That should probably happen before the
> patch gets merged.

The flexible member IS at the end of the struct and is often intended
to cover the memory in the enclosing struct. I think clang is being
overzealous with its warnings here.

Jason