linux-kernel - RE: [PATCH 1/1] x86/vdso: use the right GDT_ENTRY_CPUNODE for 32-bit getcpu() on 64-bit kernel

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, 29 Mar 2023 08:50:47 +0000
From:   "Li, Xin3" <xin3.li@...el.com>
To:     "Li, Xin3" <xin3.li@...el.com>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>
CC:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "bigeasy@...utronix.de" <bigeasy@...utronix.de>,
        "Liu, Yujie" <yujie.liu@...el.com>,
        "Kang, Shan" <shan.kang@...el.com>
Subject: RE: [PATCH 1/1] x86/vdso: use the right GDT_ENTRY_CPUNODE for 32-bit
 getcpu() on 64-bit kernel



> -----Original Message-----
> From: Li, Xin3 <xin3.li@...el.com>
> Sent: Monday, March 13, 2023 10:43 AM
> To: Hansen, Dave <dave.hansen@...el.com>; linux-kernel@...r.kernel.org;
> x86@...nel.org
> Cc: tglx@...utronix.de; mingo@...hat.com; bp@...en8.de;
> dave.hansen@...ux.intel.com; hpa@...or.com; bigeasy@...utronix.de; Liu, Yujie
> <yujie.liu@...el.com>; Kang, Shan <shan.kang@...el.com>
> Subject: RE: [PATCH 1/1] x86/vdso: use the right GDT_ENTRY_CPUNODE for 32-bit
> getcpu() on 64-bit kernel
> 
> > > +#ifndef BUILD_VDSO32_64
> > >  #define GDT_ENTRY_CPUNODE		28
> > > +#else
> > > +#define GDT_ENTRY_CPUNODE		15
> > > +#endif
> >
> > Isn't this kinda a hack?
> >
> > First, it means that we'll now have two duplicate versions of this:
> >
> > 	#define GDT_ENTRY_CPUNODE		15
> >
> > in the same file.
> >
> > Second, if any other users of fake_32bit_build.h for the VDSO show up, they'll
> > need a similar #ifdef.
> >
> > I think I'd much rather if we define all of the GDT_ENTRY_* macros in
> > *one* place, then make that *one* place depend on BUILD_VDSO32_64.
> 
> Sounds a better way, let me try.
> 
> > Also, about the *silent* failure...  Do we not have a selftest for this somewhere?
> 
> When lsl is used, we should check ZF which indicates whether the segment limit
> is loaded successfully.  Seems we need to refactor vdso_read_cpunode() a bit.

Hi Dave,

How about the following patch to address the silent failure?

diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index 794f69625780..d75ce4afff5b 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -254,7 +254,10 @@ static inline void vdso_read_cpunode(unsigned *cpu, unsigned *node)
         *
         * If RDPID is available, use it.
         */
-       alternative_io ("lsl %[seg],%[p]",
+       alternative_io ("lsl %[seg],%[p]\n"
+                       "jz 1f\n"
+                       "mov $-1,%[p]\n"
+                       "1:",
                        ".byte 0xf3,0x0f,0xc7,0xf8", /* RDPID %eax/rax */
                        X86_FEATURE_RDPID,
                        [p] "=a" (p), [seg] "r" (__CPUNODE_SEG));


And the test then reports CPU id 4095 (not a big enough #), which can be
used to indicate a failure of the lsl instruction:

~/selftests$ sudo ./run_kselftest.sh -t x86:test_vsyscall_32
TAP version 13
1..1
# selftests: x86: test_vsyscall_32
# [RUN] test gettimeofday()
#       vDSO time offsets: 0.000028 0.000000
# [OK]  vDSO gettimeofday()'s timeval was okay
# [RUN] test time()
# [OK]  vDSO time() is okay
# [RUN] getcpu() on CPU 0
# [FAIL]        vDSO reported CPU 4095 but should be 0
# [FAIL]        vDSO reported node 65535 but should be 0
# [RUN] getcpu() on CPU 1
# [FAIL]        vDSO reported CPU 4095 but should be 1
# [FAIL]        vDSO reported node 65535 but should be 0
not ok 1 selftests: x86: test_vsyscall_32 # exit=1

Thanks!
  Xin