On Wed, Apr 8, 2020 at 7:34 AM Thorsten Glaser <t.gla...@tarent.de> wrote: > > Hello, > > I’m writing to you because your name shows up on this: > > commit 45e29d119e9923ff14dfb840e3482bef1667bbfb > Author: Andy Lutomirski <l...@kernel.org> > Date: Wed Jul 3 13:34:05 2019 -0700 > > x86/syscalls: Make __X32_SYSCALL_BIT be unsigned long > > Currently, it's an int. This is bizarre. Fortunately, the code using it > still works: ~__X32_SYSCALL_BIT is also int, so, if nr is unsigned long, > then C kindly sign-extends the ~__X32_SYSCALL_BIT part, and it actually > results in the desired value. > > This is far more subtle than it deserves to be. Syscall numbers are, for > all practical purposes, unsigned long, so make __X32_SYSCALL_BIT be > unsigned long. > > Signed-off-by: Andy Lutomirski <l...@kernel.org> > Signed-off-by: Thomas Gleixner <t...@linutronix.de> > Link: > https://lkml.kernel.org/r/99b0d83ad891c67105470a1a6b63243fd63a5061.1562185330.git.l...@kernel.org > > This commit changed an uapi header, breaking userspace. Long debugging > story (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=954294 if you > are interested) short, it goes like this: > > libseccomp exposes an interface SCMP_SYS() which is designed to expand > to an int and be usable in cpp context. It redirects to the __NR_* > constants from <asm/unistd.h>. > > Example: SCMP_SYS(mmap) becomes __NR_mmap or __NR_mmap2 (depending on > the architecture). > > Now, most architectures define __NR_mmap as a mere integer number: > > asm/unistd_32.h:#define __NR_mmap 90 > asm/unistd_64.h:#define __NR_mmap 9 > > x32 differs: > > asm/unistd_x32.h:#define __NR_mmap (__X32_SYSCALL_BIT + 9) > > This construct is, thankfully, still usable in something like > #if (__NR_mmap > __NR_somethingelse) > but as __X32_SYSCALL_BIT is no longer int its type also isn’t. > > Therefore I ask you to revert this change, bringing x32 closer > to all other architectures. >
One might reasonably ask whether it makes sense for syscall nrs to be signed at all. But regardless, this breaks userspace and we should fix it. I can whip up a patch to split it into X32_SYSCALL_BIT (unsigned long) and __X32_SYSCALL_BIT (uapi, int). Thomas, etc, does this seem reasonable? (For those not following all the machinations, this change caused some userspace build failures in libseccomp and/or systemd for reasons that are vaguely silly.) > > Syscall numbers are, for > > all practical purposes, unsigned long > > Yes, except for the one purpose of the C data type of the > syscall constants exposed to userspace, they are. > > Feel free to handle __X32_SYSCALL_BIT differently in the > kernel (although even there it *will* introduce subtle > differences from other architectures), but please keep it > as int as visible from userspace. > > Thanks in advance, > //mirabilos > > PS: Please keep both me *and* the Debian bug in Cc, but > feel free to forward to relevant lists and persons; > I’m unsure where exactly to write to about this. > > @bwh: commit 45e29d119e9923ff14dfb840e3482bef1667bbfb is > literally just this… > -#define __X32_SYSCALL_BIT 0x40000000 > +#define __X32_SYSCALL_BIT 0x40000000UL > … so can you please revert it in Debian in the meantime, > even if you said you won’t spend time on this? > -- > tarent solutions GmbH > Rochusstraße 2-4, D-53123 Bonn • http://www.tarent.de/ > Tel: +49 228 54881-393 • Fax: +49 228 54881-235 > HRB 5168 (AG Bonn) • USt-ID (VAT): DE122264941 > Geschäftsführer: Dr. Stefan Barth, Kai Ebenrett, Boris Esser, Alexander Steeg > > ********** > > Mit der tarent Academy bieten wir auch Trainings und Schulungen in den > Bereichen Softwareentwicklung, Agiles Arbeiten und Zukunftstechnologien an. > > Besuchen Sie uns auf www.tarent.de/academy. Wir freuen uns auf Ihren Kontakt. > > **********
