linux-kernel - Re: [RFC][PATCH] Update -Wattribute-alias for gcc9

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] [day] [month] [year] [list]

Message-ID: <CAK8P3a1dLz3so3Z5cypOBeQN3GB-nM-RHNaeYOTEvovMrGhBPQ@mail.gmail.com>
Date:   Mon, 28 Jan 2019 15:40:31 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Bernd Edlinger <bernd.edlinger@...mail.de>
Cc:     Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
        Laura Abbott <labbott@...hat.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Martin Sebor <msebor@...il.com>
Subject: Re: [RFC][PATCH] Update -Wattribute-alias for gcc9

On Mon, Jan 28, 2019 at 2:28 PM Bernd Edlinger
<bernd.edlinger@...mail.de> wrote:
>
> On 1/25/19 1:24 PM, Bernd Edlinger wrote:
> > On 1/25/19 12:39 PM, Miguel Ojeda wrote:
> >> On Fri, Jan 25, 2019 at 11:58 AM Arnd Bergmann <arnd@...db.de> wrote:
> >>>
> >>> On Fri, Jan 25, 2019 at 11:43 AM Laura Abbott <labbott@...hat.com> wrote:
> >
> > I believe it is not intentional to break the old syntax of the
> > pragma.  There will be new -Wattribute-alias=1 and -Wattribute-alias=2
> > and -Wattribute-alias is easy to retain as an alias for -Wattribute-alias=1.
> > That is what my patch will do.
> >
>
> Okay, I committed the -Wattribute-alias patch to gcc trunk, now
> as https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=268336 .
> So there will be no need for a workaround on your side.
>
> Also fixed a few false positive -Waddress-of-packed-member warnings with
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=268118 and
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=268337 .
>
> However there remain a lot of warnings from -Waddress-of-packed-member,
> that look more or less valid, has anybody an idea how to handle
> these?

The best idea I have is "one at a time", which unfortunately
does mean a lot of work. We can degrade the warning to the
W=1 level in the kernel by disabling it by default and re-enabling
it in scripts/Makefile.extrawarn, but we probably want to address
many of them anyway. Looking at the most common examples
(listed by number of instances in Laura's build log)

     64  warning: taking address of packed member of 'struct
scif_window' may result in an unaligned pointer value
[-Waddress-of-packed-member]

Structure should not be packed at all, it's kernel internal (I hope at least,
since it contains pointers to kernel structures), and packing it does lead to
unaligned accesses.

     71  warning: converting a packed 'struct desc_struct' pointer
(alignment 1) to a 'u32' {aka 'unsigned int'} pointer (alignment 4)
may result in an unaligned pointer value [-Waddress-of-packed-member]

'__packed' does not seem to be required here. This is a structure of bit
fields, which IIRC means the layout is architecture dependent, but
it is fully packed already.

     76  warning: converting a packed 'struct opa_smp' pointer
(alignment 1) to a 'struct ib_mad_hdr' pointer (alignment 8) may
result in an unaligned pointer value [-Waddress-of-packed-member]

We cast the pointers multiple times, from aligned to packed and back to aligned,
so I assume there is no bug, but it's unclear why the structure was ever marked
as packed.

     81  warning: taking address of packed member of 'struct
adf_accel_dev' may result in an unaligned pointer value
[-Waddress-of-packed-member]

Should clearly not be packed.

    122  warning: taking address of packed member of 'struct
fc_els_flogi' may result in an unaligned pointer value
[-Waddress-of-packed-member]

I'd suggest changing it to only mark the unaligned members as packed,
not the structure itself.

    182  warning: taking address of packed member of 'struct
rtl818x_csr' may result in an unaligned pointer value
[-Waddress-of-packed-member]

Cannot be packed, since this is an mmio structure, need to
carefully rework structure layout in case there are some packed
members we never access.

   138  warning: taking address of packed member of 'struct ib_reth'
may result in an unaligned pointer value [-Waddress-of-packed-member]
    414  warning: taking address of packed member of 'struct
ib_atomic_eth' may result in an unaligned pointer value
[-Waddress-of-packed-member]

I suppose ib_u64_get should take a packed argument, it already uses
get_unaligned_be64()
internally to avoid bugs.

       Arnd