linux-kernel - Re: [PATCH 07/15] parisc: use generic sys_fanotify

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: Fri, 21 Jun 2024 05:26:27 +0000
From: LEROY Christophe <christophe.leroy2@...soprasteria.com>
To: Helge Deller <deller@....de>, Arnd Bergmann <arnd@...nel.org>,
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: Arnd Bergmann <arnd@...db.de>, Thomas Bogendoerfer
	<tsbogend@...ha.franken.de>, "linux-mips@...r.kernel.org"
	<linux-mips@...r.kernel.org>, "linux-parisc@...r.kernel.org"
	<linux-parisc@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>,
	Andreas Larsson <andreas@...sler.com>, "sparclinux@...r.kernel.org"
	<sparclinux@...r.kernel.org>, Michael Ellerman <mpe@...erman.id.au>, Nicholas
 Piggin <npiggin@...il.com>, Christophe Leroy <christophe.leroy@...roup.eu>,
	"Naveen N . Rao" <naveen.n.rao@...ux.ibm.com>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>, Brian Cain
	<bcain@...cinc.com>, "linux-hexagon@...r.kernel.org"
	<linux-hexagon@...r.kernel.org>, Guo Ren <guoren@...nel.org>,
	"linux-csky@...r.kernel.org" <linux-csky@...r.kernel.org>, Heiko Carstens
	<hca@...ux.ibm.com>, "linux-s390@...r.kernel.org"
	<linux-s390@...r.kernel.org>, Rich Felker <dalias@...c.org>, John Paul Adrian
 Glaubitz <glaubitz@...sik.fu-berlin.de>, "linux-sh@...r.kernel.org"
	<linux-sh@...r.kernel.org>, "H. Peter Anvin" <hpa@...or.com>, Alexander Viro
	<viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"libc-alpha@...rceware.org" <libc-alpha@...rceware.org>,
	"musl@...ts.openwall.com" <musl@...ts.openwall.com>, "ltp@...ts.linux.it"
	<ltp@...ts.linux.it>, Adhemerval Zanella <adhemerval.zanella@...aro.org>
Subject: Re: [PATCH 07/15] parisc: use generic sys_fanotify_mark
 implementation



Le 20/06/2024 à 23:21, Helge Deller a écrit :
> [Vous ne recevez pas souvent de courriers de deller@....de. Découvrez
> pourquoi ceci est important à
> https://aka.ms/LearnAboutSenderIdentification ]
>
> On 6/20/24 18:23, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@...db.de>
>>
>> The sys_fanotify_mark() syscall on parisc uses the reverse word order
>> for the two halves of the 64-bit argument compared to all syscalls on
>> all 32-bit architectures. As far as I can tell, the problem is that
>> the function arguments on parisc are sorted backwards (26, 25, 24, 23,
>> ...) compared to everyone else,
>
> r26 is arg0, r25 is arg1, and so on.
> I'm not sure I would call this "sorted backwards".
> I think the reason is simply that hppa is the only 32-bit big-endian
> arch left...

powerpc/32 is big-endian: r3 is arg0, r4 is arg1, ... r10 is arg7.

In case of a 64bits arg, r3 is the high part and r4 is the low part.

Christophe

>
>> so the calling conventions of using an
>> even/odd register pair in native word order result in the lower word
>> coming first in function arguments, matching the expected behavior
>> on little-endian architectures. The system call conventions however
>> ended up matching what the other 32-bit architectures do.
>>
>> A glibc cleanup in 2020 changed the userspace behavior in a way that
>> handles all architectures consistently, but this inadvertently broke
>> parisc32 by changing to the same method as everyone else.
>
> I appreciate such cleanups to make arches consistent.
> But it's bad if breakages aren't noticed or reported then...
>
>> The change made it into glibc-2.35 and subsequently into debian 12
>> (bookworm), which is the latest stable release. This means we
>> need to choose between reverting the glibc change or changing the
>> kernel to match it again, but either hange will leave some systems
>> broken.
>>
>> Pick the option that is more likely to help current and future
>> users and change the kernel to match current glibc.
>
> Agreed (assuming we have really a problem on parisc).
>
>> This also
>> means the behavior is now consistent across architectures, but
>> it breaks running new kernels with old glibc builds before 2.35.
>>
>> Link:
>> https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=d150181d73d9
>> Link:
>> https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/arch/parisc/kernel/sys_parisc.c?h=57b1dfbd5b4a39d
>> Cc: Adhemerval Zanella <adhemerval.zanella@...aro.org>
>> Signed-off-by: Arnd Bergmann <arnd@...db.de>
>> ---
>> I found this through code inspection, please double-check to make
>> sure I got the bug and the fix right.
>
> The patch looks good at first sight.
> I'll pick it up in my parisc git tree and will do some testing the
> next few days and then push forward for 6.11 when it opens....
>
> Thank you!!
>
> Helge
>
>> The alternative is to fix this by reverting glibc back to the
>> unusual behavior.
>> ---
>>   arch/parisc/Kconfig                     | 1 +
>>   arch/parisc/kernel/sys_parisc32.c       | 9 ---------
>>   arch/parisc/kernel/syscalls/syscall.tbl | 2 +-
>>   3 files changed, 2 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
>> index daafeb20f993..dc9b902de8ea 100644
>> --- a/arch/parisc/Kconfig
>> +++ b/arch/parisc/Kconfig
>> @@ -16,6 +16,7 @@ config PARISC
>>       select ARCH_HAS_UBSAN
>>       select ARCH_HAS_PTE_SPECIAL
>>       select ARCH_NO_SG_CHAIN
>> +     select ARCH_SPLIT_ARG64 if !64BIT
>>       select ARCH_SUPPORTS_HUGETLBFS if PA20
>>       select ARCH_SUPPORTS_MEMORY_FAILURE
>>       select ARCH_STACKWALK
>> diff --git a/arch/parisc/kernel/sys_parisc32.c
>> b/arch/parisc/kernel/sys_parisc32.c
>> index 2a12a547b447..826c8e51b585 100644
>> --- a/arch/parisc/kernel/sys_parisc32.c
>> +++ b/arch/parisc/kernel/sys_parisc32.c
>> @@ -23,12 +23,3 @@ asmlinkage long sys32_unimplemented(int r26, int
>> r25, int r24, int r23,
>>               current->comm, current->pid, r20);
>>       return -ENOSYS;
>>   }
>> -
>> -asmlinkage long sys32_fanotify_mark(compat_int_t fanotify_fd,
>> compat_uint_t flags,
>> -     compat_uint_t mask0, compat_uint_t mask1, compat_int_t dfd,
>> -     const char  __user * pathname)
>> -{
>> -     return sys_fanotify_mark(fanotify_fd, flags,
>> -                     ((__u64)mask1 << 32) | mask0,
>> -                      dfd, pathname);
>> -}
>> diff --git a/arch/parisc/kernel/syscalls/syscall.tbl
>> b/arch/parisc/kernel/syscalls/syscall.tbl
>> index 39e67fab7515..66dc406b12e4 100644
>> --- a/arch/parisc/kernel/syscalls/syscall.tbl
>> +++ b/arch/parisc/kernel/syscalls/syscall.tbl
>> @@ -364,7 +364,7 @@
>>   320 common  accept4                 sys_accept4
>>   321 common  prlimit64               sys_prlimit64
>>   322 common  fanotify_init           sys_fanotify_init
>> -323  common  fanotify_mark           sys_fanotify_mark
>> sys32_fanotify_mark
>> +323  common  fanotify_mark           sys_fanotify_mark
>> compat_sys_fanotify_mark
>>   324 32      clock_adjtime           sys_clock_adjtime32
>>   324 64      clock_adjtime           sys_clock_adjtime
>>   325 common  name_to_handle_at       sys_name_to_handle_at
>