linux-kernel - Re: [PATCH] Makefile.extrawarn: Move -Wunaligned-access to W=2

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:   Wed, 2 Feb 2022 09:26:04 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Masahiro Yamada <masahiroy@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        llvm@...ts.linux.dev, "# 3.4.x" <stable@...r.kernel.org>
Subject: Re: [PATCH] Makefile.extrawarn: Move -Wunaligned-access to W=2

On Wed, Feb 02, 2022 at 09:12:06AM +0100, Arnd Bergmann wrote:
> On Wed, Feb 2, 2022 at 12:22 AM Nathan Chancellor <nathan@...nel.org> wrote:
> >
> > -Wunaligned-access is a new warning in clang that is default enabled for
> > arm and arm64 under certain circumstances within the clang frontend (see
> > LLVM commit below). Under an ARCH=arm allmodconfig, there are
> > 1284 total/70 unique instances of this warning (most of the instances
> > are in header files), which is quite noisy.
> >
> > To keep the build green through CONFIG_WERROR, only allow this warning
> > with W=2, which seems appropriate according to its description:
> > "warnings which occur quite often but may still be relevant".
> >
> > This intentionally does not use the -Wno-... + -W... pattern that the
> > rest of the Makefile does because this warning is not enabled for
> > anything other than certain arm and arm64 configurations within clang so
> > it should only be "not disabled", rather than explicitly enabled, so
> > that other architectures are not disturbed by the warning.
> >
> > Cc: stable@...r.kernel.org
> > Link: https://github.com/llvm/llvm-project/commit/35737df4dcd28534bd3090157c224c19b501278a
> > Signed-off-by: Nathan Chancellor <nathan@...nel.org>
> 
> The warning seems important enough to be considered for W=1 on 32-bit arm,
> otherwise the chances of anyone actually fixing drivers for it is
> relatively slim.

Fair point, I suppose barely anyone does W=2 builds, which means we
might as well just disable it outright.

> Can you point post the (sufficiently trimmed) output that you get with
> the warning
> enabled in an allmodconfig build?

Sure thing.

Here is the trimmed version:

https://gist.github.com/nathanchance/6682e6894f75790059ca698c29212c71/raw/f63d54819afeb96f3ea0bb055096849912ac0185/trimmed.log

Here is the full output of 'make ARCH=arm LLVM=1 allmodconfig':

https://gist.github.com/nathanchance/6682e6894f75790059ca698c29212c71/raw/f63d54819afeb96f3ea0bb055096849912ac0185/full.log

> I'm not sure why this is enabled by default for arm64, which does not have
> the problem with fixup handlers.

It is not enabled for arm64 for the kernel. If I am reading the commit
right, it is only enabled for arm64 when -mno-unaligned-access is passed
or building for OpenBSD, which obviously don't apply to the kernel (see
AArch64.cpp).

For ARM, we see it in the kernel because it is enabled for any version less
than 7, according to this block in clang/lib/Driver/ToolChains/Arch/ARM.cpp:

    } else if (Triple.isOSLinux() || Triple.isOSNaCl() ||
               Triple.isOSWindows()) {
      if (VersionNum < 7) {
        Features.push_back("+strict-align");
        if (!ForAS)
          CmdArgs.push_back("-Wunaligned-access");
      }

There is this comment above this block in the source code:

    // Assume pre-ARMv6 doesn't support unaligned accesses.
    //
    // ARMv6 may or may not support unaligned accesses depending on the
    // SCTLR.U bit, which is architecture-specific. We assume ARMv6
    // Darwin and NetBSD targets support unaligned accesses, and others don't.
    //
    // ARMv7 always has SCTLR.U set to 1, but it has a new SCTLR.A bit
    // which raises an alignment fault on unaligned accesses. Linux
    // defaults this bit to 0 and handles it as a system-wide (not
    // per-process) setting. It is therefore safe to assume that ARMv7+
    // Linux targets support unaligned accesses. The same goes for NaCl
    // and Windows.
    //
    // The above behavior is consistent with GCC.

I notice that CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS under certain
conditions in arch/arm/Kconfig. Would it be worth telling clang that it
can generate unaligned accesses in those cases via -munaligned-access or
would that be too expensive? If we did, these warnings would be
eliminated for configs with CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y,
then it could be safely placed under W=1.

Cheers,
Nathan