| Message ID | 20220921065605.1051927-20-rmclure@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc: Syscall wrapper and register clearing | expand |
On Wed Sep 21, 2022 at 4:55 PM AEST, Rohan McLure wrote: > Remove explicit clearing of the high order-word of user parameters when > handling compatibility syscalls in system_call_exception. The > COMPAT_SYSCALL_DEFINEx macros handle this clearing through an > explicit cast to the signature type of the target handler. > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > Reported-by: Nicholas Piggin <npiggin@gmail.com> Thanks for digging through it to make sure things will work right without this. Some handlers look problematic without the rest of your series, right? e.g., upstream compat_sys_mmap2 has long arguments. Reviewed-by: Nicholas Piggin <npiggin@gmail.com> > --- > V6: New patch > --- > arch/powerpc/kernel/syscall.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c > index 9875486f6168..15af0ed019a7 100644 > --- a/arch/powerpc/kernel/syscall.c > +++ b/arch/powerpc/kernel/syscall.c > @@ -157,14 +157,6 @@ notrace long system_call_exception(long r3, long r4, long r5, > > if (unlikely(is_compat_task())) { > f = (void *)compat_sys_call_table[r0]; > - > - r3 &= 0x00000000ffffffffULL; > - r4 &= 0x00000000ffffffffULL; > - r5 &= 0x00000000ffffffffULL; > - r6 &= 0x00000000ffffffffULL; > - r7 &= 0x00000000ffffffffULL; > - r8 &= 0x00000000ffffffffULL; > - > } else { > f = (void *)sys_call_table[r0]; > } > -- > 2.34.1
Rohan McLure <rmclure@linux.ibm.com> writes: > Remove explicit clearing of the high order-word of user parameters when > handling compatibility syscalls in system_call_exception. The > COMPAT_SYSCALL_DEFINEx macros handle this clearing through an > explicit cast to the signature type of the target handler. Unfortunately this doesn't work. We don't have compat handlers for everything, so we end up with 64-bit values getting passsed in and things break. Our hugetlb_vs_thp selftest hits it, as seen in ftrace: 12848 mmap(0xffffffffa0000000, 16777216, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf6fb0000 12848 munmap(0xffffffffa0000000, 16777216) = -1 EINVAL (Invalid argument) But in the source the only mmap()/munmap() is of 0xa0000000. Looking at x86 they send all 32-bit syscalls via a wrapper that does the truncation (SC_IA32_REGS_TO_ARGS). So I think we could do something similar eventually and get rid of this explicit clearing. But I don't want to predicate this whole series on that, so I've dropped this patch for now, and reworked some of the following patches to keep the register clearing. cheers > diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c > index 9875486f6168..15af0ed019a7 100644 > --- a/arch/powerpc/kernel/syscall.c > +++ b/arch/powerpc/kernel/syscall.c > @@ -157,14 +157,6 @@ notrace long system_call_exception(long r3, long r4, long r5, > > if (unlikely(is_compat_task())) { > f = (void *)compat_sys_call_table[r0]; > - > - r3 &= 0x00000000ffffffffULL; > - r4 &= 0x00000000ffffffffULL; > - r5 &= 0x00000000ffffffffULL; > - r6 &= 0x00000000ffffffffULL; > - r7 &= 0x00000000ffffffffULL; > - r8 &= 0x00000000ffffffffULL; > - > } else { > f = (void *)sys_call_table[r0]; > } > -- > 2.34.1
diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c index 9875486f6168..15af0ed019a7 100644 --- a/arch/powerpc/kernel/syscall.c +++ b/arch/powerpc/kernel/syscall.c @@ -157,14 +157,6 @@ notrace long system_call_exception(long r3, long r4, long r5, if (unlikely(is_compat_task())) { f = (void *)compat_sys_call_table[r0]; - - r3 &= 0x00000000ffffffffULL; - r4 &= 0x00000000ffffffffULL; - r5 &= 0x00000000ffffffffULL; - r6 &= 0x00000000ffffffffULL; - r7 &= 0x00000000ffffffffULL; - r8 &= 0x00000000ffffffffULL; - } else { f = (void *)sys_call_table[r0]; }
Remove explicit clearing of the high order-word of user parameters when handling compatibility syscalls in system_call_exception. The COMPAT_SYSCALL_DEFINEx macros handle this clearing through an explicit cast to the signature type of the target handler. Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> Reported-by: Nicholas Piggin <npiggin@gmail.com> --- V6: New patch --- arch/powerpc/kernel/syscall.c | 8 -------- 1 file changed, 8 deletions(-)