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  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, 14 Aug 2020 23:19:36 +0200
From:   Sedat Dilek <sedat.dilek@...il.com>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Peter Collingbourne <pcc@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Zhenzhong Duan <zhenzhong.duan@...cle.com>,
        Kees Cook <keescook@...omium.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Juergen Gross <jgross@...e.com>,
        Andy Lutomirski <luto@...nel.org>,
        Andrew Cooper <andrew.cooper3@...rix.com>,
        LKML <linux-kernel@...r.kernel.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>
Subject: Re: [PATCH] x86: work around clang IAS bug referencing __force_order

On Fri, Aug 14, 2020 at 7:29 PM Sedat Dilek <sedat.dilek@...il.com> wrote:
>
> On Tue, Aug 4, 2020 at 2:09 AM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@...glegroups.com> wrote:
> >
> > On Wed, May 27, 2020 at 6:53 AM Arnd Bergmann <arnd@...db.de> wrote:
> > >
> > > When using the clang integrated assembler, we get a reference
> > > to __force_order that should normally get ignored in a few
> > > rare cases:
> > >
> > > ERROR: modpost: "__force_order" [drivers/cpufreq/powernow-k6.ko] undefined!
> > >
> > > Add a 'static' definition so any file in which this happens can
> > > have a local copy.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@...db.de>
> >
> > Hi Arnd,
> > Looks like
> > $ ARCH=i386 make CC=clang LLVM_IAS=1 -j71
> > defconfig+CONFIG_X86_POWERNOW_K6=m
> > is the simplest reproducer.
> >
> > If I run
> > $ llvm-readelf -s drivers/cpufreq/powernow-k6.o | grep __force_order
> >     39: 00000000     0 NOTYPE  GLOBAL DEFAULT   UND __force_order
> > we can indeed see an undefined reference to __force_order.
> >
> > If I build the .s file via
> > $ ARCH=i386 make CC=clang LLVM_IAS=1 -j71 drivers/cpufreq/powernow-k6.s
> > the only reference I see to __force_order is:
> > 979   .addrsig_sym __force_order
> >
> > which is created by Clang's implicit -faddr-sig.  If I disable that
> > for this file via:
> >
> > ```diff
> > diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> > index f1b7e3dd6e5d..87d655d5af49 100644
> > --- a/drivers/cpufreq/Makefile
> > +++ b/drivers/cpufreq/Makefile
> > @@ -28,6 +28,9 @@ obj-$(CONFIG_X86_ACPI_CPUFREQ)                +=
> > acpi-cpufreq.o
> >  obj-$(CONFIG_X86_POWERNOW_K8)          += powernow-k8.o
> >  obj-$(CONFIG_X86_PCC_CPUFREQ)          += pcc-cpufreq.o
> >  obj-$(CONFIG_X86_POWERNOW_K6)          += powernow-k6.o
> > +ifdef CONFIG_CC_IS_CLANG
> > +CFLAGS_powernow-k6.o                   += -fno-addrsig
> > +endif
> >  obj-$(CONFIG_X86_POWERNOW_K7)          += powernow-k7.o
> >  obj-$(CONFIG_X86_LONGHAUL)             += longhaul.o
> >  obj-$(CONFIG_X86_E_POWERSAVER)         += e_powersaver.o
> > ```
> > then the module links without error, and we get no hits for
> > __force_order from llvm-readelf -s.  This makes me think there may be
> > a bug in Clang generating address significance tables for global
> > variables that are otherwise unused, resulting in such linkage
> > failures. +pcc@ for that.
> >
> > I ran a creduce job on drivers/cpufreq/powernow-k6.i where I'd compile
> > twice, one with the implicit default value of -faddr-sig and look for
> > the undefined __force_order, and again with -fno-addrsig and ensure
> > there was no undefined __force_order, which coughed up:
> > extern int __force_order;
> > int a(void) { asm("" : "=m"(__force_order)); return 0; }
> > as the bare minimum for an address significant table.
> > https://godbolt.org/z/cjfaqM
> >
> > I'll bet this is coming from the call to read_cr0() in
> > powernow_k6_set_cpu_multiplier().  If __force_order is defined in
> > arch/x86/boot/compressed/pgtable_64.c, then I'm not sure it's a good
> > idea to build drivers/cpufreq/powernow-k6.c as a kernel module
> > (CONFIG_X86_POWERNOW_K6=m) vs statically compiled in.  Wouldn't
> > __force_order need to be EXPORT'ed for kernel modules to use it
> > safely?
> >
> > > ---
> > >  arch/x86/boot/compressed/pgtable_64.c | 2 ++
> > >  arch/x86/include/asm/special_insns.h  | 7 +++++++
> > >  2 files changed, 9 insertions(+)
> > >
> > > diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
> > > index c8862696a47b..8595194cea41 100644
> > > --- a/arch/x86/boot/compressed/pgtable_64.c
> > > +++ b/arch/x86/boot/compressed/pgtable_64.c
> > > @@ -12,7 +12,9 @@
> > >   * It is not referenced from the code, but GCC < 5 with -fPIE would fail
> > >   * due to an undefined symbol. Define it to make these ancient GCCs work.
> > >   */
> > > +#ifndef CONFIG_CC_IS_CLANG
> > >  unsigned long __force_order;
> > > +#endif
> > >
> > >  #define BIOS_START_MIN         0x20000U        /* 128K, less than this is insane */
> > >  #define BIOS_START_MAX         0x9f000U        /* 640K, absolute maximum */
> > > diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
> > > index 82436cb04ccf..7081e587c1ea 100644
> > > --- a/arch/x86/include/asm/special_insns.h
> > > +++ b/arch/x86/include/asm/special_insns.h
> > > @@ -16,8 +16,15 @@
> > >   * A memory clobber would solve the problem, but would prevent reordering of
> > >   * all loads stores around it, which can hurt performance. Solution is to
> > >   * use a variable and mimic reads and writes to it to enforce serialization
> > > + *
> > > + * Clang sometimes fails to kill the reference to the dummy variable, so
> > > + * provide an actual copy.
> > >   */
> > > +#ifdef CONFIG_CC_IS_CLANG
> > > +static unsigned long __force_order;
> > > +#else
> > >  extern unsigned long __force_order;
> > > +#endif
> > >
> > >  void native_write_cr0(unsigned long val);
> > >
> > > --
> > > 2.26.2
> > >
>
> Thanks for the proposal.
>
> I have adapted it to fit my patchset against Linux v5.8.
>
> Both Debian's GCC-10 and a snapshot version of LLVM toolchain
> v11.0.0-rc1+ seems to be OK.
>
> MAKE_OPTS="V=1 -j3 CC=gcc-10 LD=ld.bfd"
> make $MAKE_OPTS arch/x86/kernel/cpu/common.o
>
> MAKE_OPTS="V=1 -j3 HOSTCC=clang-11 HOSTCXX=clang++-11 HOSTLD=ld.lld-11
> HOSTAR=llvm-ar-11 CC=clang-11 LD=ld.lld-11 AR=llvm-ar-11 NM=llvm-nm-11
> OBJCOPY=llvm-objcopy-11 OBJDUMP=llvm-objdump-11 OBJSIZE=llvm-size-11
> READELF=llvm-readelf-11 STRIP=llvm-strip-11 LLVM_IAS=1"
> make $MAKE_OPTS arch/x86/kernel/cpu/common.o
>
> I can send both object files if desired.
>
> I will do a full kernel-build to see if I am able to build the
> VirtualBox out-of-tree kernel-modules.
>

Yupp, OK.

I was able to boot FreeDOS 1.2 VM in VirtualBox GUI.

My kernel-config and dmesg-output are attached.

- Sedat -

P.S.: Logs

root@...za:~# systemctl status virtualbox.service
● virtualbox.service - LSB: VirtualBox Linux kernel module
     Loaded: loaded (/etc/init.d/virtualbox; generated)
     Active: inactive (dead)
       Docs: man:systemd-sysv-generator(8)

root@...za:~# systemctl start virtualbox.service

root@...za:~# systemctl status virtualbox.service
● virtualbox.service - LSB: VirtualBox Linux kernel module
     Loaded: loaded (/etc/init.d/virtualbox; generated)
     Active: active (exited) since Fri 2020-08-14 23:04:08 CEST; 1s ago
       Docs: man:systemd-sysv-generator(8)
    Process: 2289 ExecStart=/etc/init.d/virtualbox start (code=exited,
status=0/SUCCESS)

Aug 14 23:04:07 iniza systemd[1]: Starting LSB: VirtualBox Linux
kernel module...
Aug 14 23:04:08 iniza virtualbox[2289]: Loading VirtualBox kernel
modules... vboxdrv vboxnetflt vboxnetadp.
Aug 14 23:04:08 iniza systemd[1]: Started LSB: VirtualBox Linux kernel module.

root@...za:~# dmesg -T | tail
[Fr Aug 14 23:03:31 2020] wlp1s0: associate with 50:d4:f7:2e:17:da (try 1/3)
[Fr Aug 14 23:03:31 2020] wlp1s0: RX AssocResp from 50:d4:f7:2e:17:da
(capab=0x411 status=0 aid=1)
[Fr Aug 14 23:03:31 2020] wlp1s0: associated
[Fr Aug 14 23:03:31 2020] IPv6: ADDRCONF(NETDEV_CHANGE): wlp1s0: link
becomes ready
[Fr Aug 14 23:04:08 2020] vboxdrv: loading out-of-tree module taints kernel.
[Fr Aug 14 23:04:08 2020] vboxdrv: Found 4 processor cores
[Fr Aug 14 23:04:08 2020] vboxdrv: TSC mode is Invariant, tentative
frequency 1600198501 Hz
[Fr Aug 14 23:04:08 2020] vboxdrv: Successfully loaded version
6.1.12_Debian (interface 0x002d0001)
[Fr Aug 14 23:04:08 2020] VBoxNetFlt: Successfully started.
[Fr Aug 14 23:04:08 2020] VBoxNetAdp: Successfully started.

root@...za:~# lsmod | grep vbox
vboxnetadp             28672  0
vboxnetflt             32768  0
vboxdrv               532480  2 vboxnetadp,vboxnetflt

- EOT -

View attachment "dmesg-T_5.8.1-8-amd64-gcc10-bfd.txt" of type "text/plain" (73499 bytes)

Download attachment "config-5.8.1-8-amd64-gcc10-bfd" of type "application/octet-stream" (232998 bytes)

Powered by blists - more mailing lists