[musl] Re: [PATCH 07/15] parisc: use generic sys_fanotify_mark implementation

[LEROY Christophe <christophe.leroy2@cs-soprasteria.com>]

Le 20/06/2024 à 23:21, Helge Deller a écrit :
> [Vous ne recevez pas souvent de courriers de deller@gmx.de. Découvrez
> pourquoi ceci est important à
> https://aka.ms/LearnAboutSenderIdentification ]
>
> On 6/20/24 18:23, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.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@linaro.org>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.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
>