linux-kernel - Re: [PATCH] arm64/efi: Move variable assignments after SECTIONS

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]

Message-ID: <20190814161904.55jgaxnhd4ujyh2h@willie-the-truck>
Date:   Wed, 14 Aug 2019 17:19:04 +0100
From:   Will Deacon <will@...nel.org>
To:     Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc:     Kees Cook <keescook@...omium.org>,
        Nathan Chancellor <natechancellor@...il.com>,
        Catalin Marinas <catalin.marinas@....com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>,
        Fangrui Song <maskray@...gle.com>,
        Peter Smith <peter.smith@...aro.org>
Subject: Re: [PATCH] arm64/efi: Move variable assignments after SECTIONS

On Wed, Aug 14, 2019 at 07:14:42PM +0300, Ard Biesheuvel wrote:
> On Wed, 14 Aug 2019 at 02:04, Kees Cook <keescook@...omium.org> wrote:
> >
> > It seems that LLVM's linker does not correctly handle variable assignments
> > involving section positions that are updated during the SECTIONS
> > parsing. Commit aa69fb62bea1 ("arm64/efi: Mark __efistub_stext_offset as
> > an absolute symbol explicitly") ran into this too, but found a different
> > workaround.
> >
> > However, this was not enough, as other variables were also miscalculated
> > which manifested as boot failures under UEFI where __efistub__end was
> > not taking the correct _end value (they should be the same):
> >
> > $ ld.lld -EL -maarch64elf --no-undefined -X -shared \
> >         -Bsymbolic -z notext -z norelro --no-apply-dynamic-relocs \
> >         -o vmlinux.lld -T poc.lds --whole-archive vmlinux.o && \
> >   readelf -Ws vmlinux.lld | egrep '\b(__efistub_|)_end\b'
> > 368272: ffff000002218000     0 NOTYPE  LOCAL  HIDDEN    38 __efistub__end
> > 368322: ffff000012318000     0 NOTYPE  GLOBAL DEFAULT   38 _end
> >
> > $ aarch64-linux-gnu-ld.bfd -EL -maarch64elf --no-undefined -X -shared \
> >         -Bsymbolic -z notext -z norelro --no-apply-dynamic-relocs \
> >         -o vmlinux.bfd -T poc.lds --whole-archive vmlinux.o && \
> >   readelf -Ws vmlinux.bfd | egrep '\b(__efistub_|)_end\b'
> > 338124: ffff000012318000     0 NOTYPE  LOCAL  DEFAULT  ABS __efistub__end
> > 383812: ffff000012318000     0 NOTYPE  GLOBAL DEFAULT 15325 _end
> >
> > To work around this, all of the __efistub_-prefixed variable assignments
> > need to be moved after the linker script's SECTIONS entry. As it turns
> > out, this also solves the problem fixed in commit aa69fb62bea1, so those
> > changes are reverted here.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/634
> > Link: https://bugs.llvm.org/show_bug.cgi?id=42990
> > Signed-off-by: Kees Cook <keescook@...omium.org>
> 
> Although it is slightly disappointing that we need to work around this
> kind of bugs when adding support for a new toolchain, I don't see
> anything wrong with this patch, so
> 
> Acked-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>

Yup, it's gross, but I'll queue it with your ack.

Will