linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs
@ 2022-10-12  3:53 Nicholas Piggin
  2022-10-12  3:53 ` [PATCH 2/2] powerpc: move sync_file_range2 compat definition Nicholas Piggin
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Nicholas Piggin @ 2022-10-12  3:53 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Jason A . Donenfeld, Nicholas Piggin

powerpc 32-bit system call (and function) calling convention for 64-bit
arguments requires the next available odd-pair (two sequential registers
with the first being odd-numbered) from the standard register argument
allocation.

The first argument register is r3, so a 64-bit argument that appears at
an even position in the argument list must skip a register (unless there
were preceeding 64-bit arguments, which might throw things off). This
requires non-standard compat definitions to deal with the holes in the
argument register allocation.

With pt_regs syscall wrappers which use a standard mapper to map pt_regs
GPRs to function arguments, 32-bit kernels hit the same basic problem,
the standard definitions don't cope with the unused argument registers.

Fix this by having 32-bit kernels share those syscall definitions with
compat.

Thanks to Jason for spending a lot of time finding and bisecting this and
developing a trivial reproducer. The perfect bug report.

Reported-by: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Since posting the patch in the bug thread:
- dropped sync_file_range2 from the ppc32 syscalls.
- updated some comments on sys_ppc32.c, which now has a different
  purpose.
- still have not explicitly tested all convrted syscalls on ppc32
  (I don't have a great ppc32 environment to test such things,
  will have to try rectify that...).
- Build fixes from mpe.
- Thanks to jms and mpe for helping with debugging.

Thanks,
Nick

 arch/powerpc/include/asm/syscalls.h      | 16 ++++++++++
 arch/powerpc/kernel/Makefile             |  1 +
 arch/powerpc/kernel/sys_ppc32.c          | 38 ++++++++++++++++++------
 arch/powerpc/kernel/syscalls/syscall.tbl | 16 ++++++----
 4 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index 9840d572da55..a1142496cd58 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -89,6 +89,22 @@ long compat_sys_rt_sigreturn(void);
  * responsible for combining parameter pairs.
  */
 
+#ifdef CONFIG_PPC32
+long sys_ppc_pread64(unsigned int fd,
+		     char __user *ubuf, compat_size_t count,
+		     u32 reg6, u32 pos1, u32 pos2);
+long sys_ppc_pwrite64(unsigned int fd,
+		      const char __user *ubuf, compat_size_t count,
+		      u32 reg6, u32 pos1, u32 pos2);
+long sys_ppc_readahead(int fd, u32 r4,
+		       u32 offset1, u32 offset2, u32 count);
+long sys_ppc_truncate64(const char __user *path, u32 reg4,
+		        unsigned long len1, unsigned long len2);
+long sys_ppc_ftruncate64(unsigned int fd, u32 reg4,
+			 unsigned long len1, unsigned long len2);
+long sys_ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
+			 size_t len, int advice);
+#endif
 #ifdef CONFIG_COMPAT
 long compat_sys_mmap2(unsigned long addr, size_t len,
 		      unsigned long prot, unsigned long flags,
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 1f121c188805..d382564034a7 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -73,6 +73,7 @@ obj-y				:= cputable.o syscalls.o \
 obj-y				+= ptrace/
 obj-$(CONFIG_PPC64)		+= setup_64.o irq_64.o\
 				   paca.o nvram_64.o note.o
+obj-$(CONFIG_PPC32)		+= sys_ppc32.o
 obj-$(CONFIG_COMPAT)		+= sys_ppc32.o signal_32.o
 obj-$(CONFIG_VDSO32)		+= vdso32_wrapper.o
 obj-$(CONFIG_PPC_WATCHDOG)	+= watchdog.o
diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
index dcc3c9fd4cfd..6bdfe11a7689 100644
--- a/arch/powerpc/kernel/sys_ppc32.c
+++ b/arch/powerpc/kernel/sys_ppc32.c
@@ -1,13 +1,23 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * sys_ppc32.c: Conversion between 32bit and 64bit native syscalls.
+ * sys_ppc32.c: 32-bit system calls with complex calling conventions.
  *
  * Copyright (C) 2001 IBM
  * Copyright (C) 1997,1998 Jakub Jelinek (jj@sunsite.mff.cuni.cz)
  * Copyright (C) 1997 David S. Miller (davem@caip.rutgers.edu)
  *
- * These routines maintain argument size conversion between 32bit and 64bit
- * environment.
+ * 32-bit system calls with 64-bit arguments pass those in register pairs.
+ * This must be specially dealt with on 64-bit kernels. The compat_arg_u64_dual
+ * in generic compat syscalls is not always usable because the register
+ * pairing is constrained depending on preceeding arguments.
+ *
+ * An analogous problem exists on 32-bit kernels with ARCH_HAS_SYSCALL_WRAPPER,
+ * the defined system call functions take the pt_regs as an argument, and there
+ * is a mapping macro which maps registers to arguments
+ * (SC_POWERPC_REGS_TO_ARGS) which also does not deal with these 64-bit
+ * arguments.
+ *
+ * This file contains these system calls.
  */
 
 #include <linux/kernel.h>
@@ -47,7 +57,17 @@
 #include <asm/syscalls.h>
 #include <asm/switch_to.h>
 
-COMPAT_SYSCALL_DEFINE6(ppc_pread64,
+#ifdef CONFIG_PPC32
+#define PPC32_SYSCALL_DEFINE4	SYSCALL_DEFINE4
+#define PPC32_SYSCALL_DEFINE5	SYSCALL_DEFINE5
+#define PPC32_SYSCALL_DEFINE6	SYSCALL_DEFINE6
+#else
+#define PPC32_SYSCALL_DEFINE4	COMPAT_SYSCALL_DEFINE4
+#define PPC32_SYSCALL_DEFINE5	COMPAT_SYSCALL_DEFINE5
+#define PPC32_SYSCALL_DEFINE6	COMPAT_SYSCALL_DEFINE6
+#endif
+
+PPC32_SYSCALL_DEFINE6(ppc_pread64,
 		       unsigned int, fd,
 		       char __user *, ubuf, compat_size_t, count,
 		       u32, reg6, u32, pos1, u32, pos2)
@@ -55,7 +75,7 @@ COMPAT_SYSCALL_DEFINE6(ppc_pread64,
 	return ksys_pread64(fd, ubuf, count, merge_64(pos1, pos2));
 }
 
-COMPAT_SYSCALL_DEFINE6(ppc_pwrite64,
+PPC32_SYSCALL_DEFINE6(ppc_pwrite64,
 		       unsigned int, fd,
 		       const char __user *, ubuf, compat_size_t, count,
 		       u32, reg6, u32, pos1, u32, pos2)
@@ -63,28 +83,28 @@ COMPAT_SYSCALL_DEFINE6(ppc_pwrite64,
 	return ksys_pwrite64(fd, ubuf, count, merge_64(pos1, pos2));
 }
 
-COMPAT_SYSCALL_DEFINE5(ppc_readahead,
+PPC32_SYSCALL_DEFINE5(ppc_readahead,
 		       int, fd, u32, r4,
 		       u32, offset1, u32, offset2, u32, count)
 {
 	return ksys_readahead(fd, merge_64(offset1, offset2), count);
 }
 
-COMPAT_SYSCALL_DEFINE4(ppc_truncate64,
+PPC32_SYSCALL_DEFINE4(ppc_truncate64,
 		       const char __user *, path, u32, reg4,
 		       unsigned long, len1, unsigned long, len2)
 {
 	return ksys_truncate(path, merge_64(len1, len2));
 }
 
-COMPAT_SYSCALL_DEFINE4(ppc_ftruncate64,
+PPC32_SYSCALL_DEFINE4(ppc_ftruncate64,
 		       unsigned int, fd, u32, reg4,
 		       unsigned long, len1, unsigned long, len2)
 {
 	return ksys_ftruncate(fd, merge_64(len1, len2));
 }
 
-COMPAT_SYSCALL_DEFINE6(ppc32_fadvise64,
+PPC32_SYSCALL_DEFINE6(ppc32_fadvise64,
 		       int, fd, u32, unused, u32, offset1, u32, offset2,
 		       size_t, len, int, advice)
 {
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 2bca64f96164..e9e0df4f9a61 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -228,8 +228,10 @@
 176	64	rt_sigtimedwait			sys_rt_sigtimedwait
 177	nospu 	rt_sigqueueinfo			sys_rt_sigqueueinfo		compat_sys_rt_sigqueueinfo
 178	nospu 	rt_sigsuspend			sys_rt_sigsuspend		compat_sys_rt_sigsuspend
-179	common	pread64				sys_pread64			compat_sys_ppc_pread64
-180	common	pwrite64			sys_pwrite64			compat_sys_ppc_pwrite64
+179	32	pread64				sys_ppc_pread64			compat_sys_ppc_pread64
+179	64	pread64				sys_pread64
+180	32	pwrite64			sys_ppc_pwrite64		compat_sys_ppc_pwrite64
+180	64	pwrite64			sys_pwrite64
 181	common	chown				sys_chown
 182	common	getcwd				sys_getcwd
 183	common	capget				sys_capget
@@ -242,10 +244,11 @@
 188	common 	putpmsg				sys_ni_syscall
 189	nospu	vfork				sys_vfork
 190	common	ugetrlimit			sys_getrlimit			compat_sys_getrlimit
-191	common	readahead			sys_readahead			compat_sys_ppc_readahead
+191	32	readahead			sys_ppc_readahead		compat_sys_ppc_readahead
+191	64	readahead			sys_readahead
 192	32	mmap2				sys_mmap2			compat_sys_mmap2
-193	32	truncate64			sys_truncate64			compat_sys_ppc_truncate64
-194	32	ftruncate64			sys_ftruncate64			compat_sys_ppc_ftruncate64
+193	32	truncate64			sys_ppc_truncate64		compat_sys_ppc_truncate64
+194	32	ftruncate64			sys_ppc_ftruncate64		compat_sys_ppc_ftruncate64
 195	32	stat64				sys_stat64
 196	32	lstat64				sys_lstat64
 197	32	fstat64				sys_fstat64
@@ -288,7 +291,8 @@
 230	common	io_submit			sys_io_submit			compat_sys_io_submit
 231	common	io_cancel			sys_io_cancel
 232	nospu	set_tid_address			sys_set_tid_address
-233	common	fadvise64			sys_fadvise64			compat_sys_ppc32_fadvise64
+233	32	fadvise64			sys_ppc32_fadvise64		compat_sys_ppc32_fadvise64
+233	64	fadvise64			sys_fadvise64
 234	nospu	exit_group			sys_exit_group
 235	nospu	lookup_dcookie			sys_lookup_dcookie		compat_sys_lookup_dcookie
 236	common	epoll_create			sys_epoll_create
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] powerpc: move sync_file_range2 compat definition
  2022-10-12  3:53 [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs Nicholas Piggin
@ 2022-10-12  3:53 ` Nicholas Piggin
  2022-10-12 10:21   ` Arnd Bergmann
  2022-10-12  7:56 ` [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs Andrew Donnellan
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Nicholas Piggin @ 2022-10-12  3:53 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Jason A . Donenfeld, Nicholas Piggin

sync_file_range2 is not a special unaligned-odd-pair calling convention
syscall, it's just a regular one that does not have a generic compat
definition. Move it out of sys_ppc32.c and into syscalls.c.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
This one doesn't fix anything and is not required for the previous
fix, so it could be merged later. Now that we've repurposed sys_ppc32.c
for the difficult syscalls and compat syscalls live all over the kernel
now anyway, IMO it's makes things less confusing to move this.

Thanks,
Nick

 arch/powerpc/include/asm/syscalls.h | 23 ++++++++++++-----------
 arch/powerpc/kernel/sys_ppc32.c     | 11 -----------
 arch/powerpc/kernel/syscalls.c      | 11 +++++++++++
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index a1142496cd58..810ce45d670b 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -79,8 +79,20 @@ long compat_sys_old_getrlimit(unsigned int resource,
 			      struct compat_rlimit __user *rlim);
 long compat_sys_sigreturn(void);
 long compat_sys_rt_sigreturn(void);
+long compat_sys_ppc_sync_file_range2(int fd, unsigned int flags,
+				     unsigned int offset1,
+				     unsigned int offset2,
+				     unsigned int nbytes1,
+				     unsigned int nbytes2);
 #endif /* CONFIG_COMPAT */
 
+#if defined(CONFIG_PPC32) || defined(CONFIG_COMPAT)
+long sys_ppc_fadvise64_64(int fd, int advice,
+			  u32 offset_high, u32 offset_low,
+			  u32 len_high, u32 len_low);
+#endif
+
+
 /*
  * Architecture specific signatures required by long long munging:
  * The 32 bit ABI passes long longs in an odd even register pair.
@@ -123,19 +135,8 @@ long compat_sys_ppc_ftruncate64(unsigned int fd, u32 reg4,
 				unsigned long len1, unsigned long len2);
 long compat_sys_ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
 				size_t len, int advice);
-long compat_sys_ppc_sync_file_range2(int fd, unsigned int flags,
-				     unsigned int offset1,
-				     unsigned int offset2,
-				     unsigned int nbytes1,
-				     unsigned int nbytes2);
 #endif /* CONFIG_COMPAT */
 
-#if defined(CONFIG_PPC32) || defined(CONFIG_COMPAT)
-long sys_ppc_fadvise64_64(int fd, int advice,
-			  u32 offset_high, u32 offset_low,
-			  u32 len_high, u32 len_low);
-#endif
-
 #else
 
 #define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, native)
diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
index 6bdfe11a7689..53927abcda82 100644
--- a/arch/powerpc/kernel/sys_ppc32.c
+++ b/arch/powerpc/kernel/sys_ppc32.c
@@ -111,14 +111,3 @@ PPC32_SYSCALL_DEFINE6(ppc32_fadvise64,
 	return ksys_fadvise64_64(fd, merge_64(offset1, offset2), len,
 				 advice);
 }
-
-COMPAT_SYSCALL_DEFINE6(ppc_sync_file_range2,
-		       int, fd, unsigned int, flags,
-		       unsigned int, offset1, unsigned int, offset2,
-		       unsigned int, nbytes1, unsigned int, nbytes2)
-{
-	loff_t offset = merge_64(offset1, offset2);
-	loff_t nbytes = merge_64(nbytes1, nbytes2);
-
-	return ksys_sync_file_range(fd, offset, nbytes, flags);
-}
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 68ebb23a5af4..8bdc41879045 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -97,6 +97,17 @@ COMPAT_SYSCALL_DEFINE1(ppc64_personality, unsigned long, personality)
 {
 	return do_ppc64_personality(personality);
 }
+
+COMPAT_SYSCALL_DEFINE6(ppc_sync_file_range2,
+		       int, fd, unsigned int, flags,
+		       unsigned int, offset1, unsigned int, offset2,
+		       unsigned int, nbytes1, unsigned int, nbytes2)
+{
+	loff_t offset = merge_64(offset1, offset2);
+	loff_t nbytes = merge_64(nbytes1, nbytes2);
+
+	return ksys_sync_file_range(fd, offset, nbytes, flags);
+}
 #endif /* CONFIG_COMPAT */
 #endif /* CONFIG_PPC64 */
 
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs
  2022-10-12  3:53 [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs Nicholas Piggin
  2022-10-12  3:53 ` [PATCH 2/2] powerpc: move sync_file_range2 compat definition Nicholas Piggin
@ 2022-10-12  7:56 ` Andrew Donnellan
  2022-10-12 10:15 ` Arnd Bergmann
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Andrew Donnellan @ 2022-10-12  7:56 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Jason A . Donenfeld

On Wed, 2022-10-12 at 13:53 +1000, Nicholas Piggin wrote:
> powerpc 32-bit system call (and function) calling convention for 64-
> bit
> arguments requires the next available odd-pair (two sequential
> registers
> with the first being odd-numbered) from the standard register
> argument
> allocation.
> 
> The first argument register is r3, so a 64-bit argument that appears
> at
> an even position in the argument list must skip a register (unless
> there
> were preceeding 64-bit arguments, which might throw things off). This
> requires non-standard compat definitions to deal with the holes in
> the
> argument register allocation.
> 
> With pt_regs syscall wrappers which use a standard mapper to map
> pt_regs
> GPRs to function arguments, 32-bit kernels hit the same basic
> problem,
> the standard definitions don't cope with the unused argument
> registers.
> 
> Fix this by having 32-bit kernels share those syscall definitions
> with
> compat.
> 
> Thanks to Jason for spending a lot of time finding and bisecting this
> and
> developing a trivial reproducer. The perfect bug report.
> 
> Reported-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Fixes: 7e92e01b72452 ("powerpc: Provide syscall wrapper")

-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd@linux.ibm.com   IBM Australia Limited


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs
  2022-10-12  3:53 [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs Nicholas Piggin
  2022-10-12  3:53 ` [PATCH 2/2] powerpc: move sync_file_range2 compat definition Nicholas Piggin
  2022-10-12  7:56 ` [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs Andrew Donnellan
@ 2022-10-12 10:15 ` Arnd Bergmann
  2022-10-14 23:21 ` (subset) " Michael Ellerman
  2022-10-30 17:03 ` Andreas Schwab
  4 siblings, 0 replies; 7+ messages in thread
From: Arnd Bergmann @ 2022-10-12 10:15 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Jason A . Donenfeld

On Wed, Oct 12, 2022, at 5:53 AM, Nicholas Piggin wrote:
> powerpc 32-bit system call (and function) calling convention for 64-bit
> arguments requires the next available odd-pair (two sequential registers
> with the first being odd-numbered) from the standard register argument
> allocation.
>
> The first argument register is r3, so a 64-bit argument that appears at
> an even position in the argument list must skip a register (unless there
> were preceeding 64-bit arguments, which might throw things off). This
> requires non-standard compat definitions to deal with the holes in the
> argument register allocation.
>
> With pt_regs syscall wrappers which use a standard mapper to map pt_regs
> GPRs to function arguments, 32-bit kernels hit the same basic problem,
> the standard definitions don't cope with the unused argument registers.
>
> Fix this by having 32-bit kernels share those syscall definitions with
> compat.
>
> Thanks to Jason for spending a lot of time finding and bisecting this and
> developing a trivial reproducer. The perfect bug report.
>
> Reported-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

This looks like a good approach to fix the regression. Comments
below only for additional thoughts, don't let that hold up
merging.

> +#ifdef CONFIG_PPC32
> +long sys_ppc_pread64(unsigned int fd,
> +		     char __user *ubuf, compat_size_t count,
> +		     u32 reg6, u32 pos1, u32 pos2);
> +long sys_ppc_pwrite64(unsigned int fd,
> +		      const char __user *ubuf, compat_size_t count,
> +		      u32 reg6, u32 pos1, u32 pos2);
> +long sys_ppc_readahead(int fd, u32 r4,
> +		       u32 offset1, u32 offset2, u32 count);
> +long sys_ppc_truncate64(const char __user *path, u32 reg4,
> +		        unsigned long len1, unsigned long len2);
> +long sys_ppc_ftruncate64(unsigned int fd, u32 reg4,
> +			 unsigned long len1, unsigned long len2);
> +long sys_ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2,
> +			 size_t len, int advice);
> +#endif

In general, I would leave out the #ifdef here and always declare
the functions, but it doesn't really matter.

>   *
> - * These routines maintain argument size conversion between 32bit and 64bit
> - * environment.
> + * 32-bit system calls with 64-bit arguments pass those in register pairs.
> + * This must be specially dealt with on 64-bit kernels. The compat_arg_u64_dual
> + * in generic compat syscalls is not always usable because the register
> + * pairing is constrained depending on preceeding arguments.
> + *
> + * An analogous problem exists on 32-bit kernels with ARCH_HAS_SYSCALL_WRAPPER,
> + * the defined system call functions take the pt_regs as an argument, and there
> + * is a mapping macro which maps registers to arguments
> + * (SC_POWERPC_REGS_TO_ARGS) which also does not deal with these 64-bit
> + * arguments.
> + *
> + * This file contains these system calls.

It would be nice to eventually move these next to the regular system
call definitions, with more generic naming and #ifdef checks. It looks
like these are the exact same ones that we have in
arch/arm64/kernel/sys32.c and arch/mips/kernel/linux32.c,
while the other five (x86, s390, sparc, riscv, parisc) use the
version without padding that was recently added as the generic
compat syscall set.

> @@ -47,7 +57,17 @@
>  #include <asm/syscalls.h>
>  #include <asm/switch_to.h>
> 
> -COMPAT_SYSCALL_DEFINE6(ppc_pread64,
> +#ifdef CONFIG_PPC32
> +#define PPC32_SYSCALL_DEFINE4	SYSCALL_DEFINE4
> +#define PPC32_SYSCALL_DEFINE5	SYSCALL_DEFINE5
> +#define PPC32_SYSCALL_DEFINE6	SYSCALL_DEFINE6
> +#else
> +#define PPC32_SYSCALL_DEFINE4	COMPAT_SYSCALL_DEFINE4
> +#define PPC32_SYSCALL_DEFINE5	COMPAT_SYSCALL_DEFINE5
> +#define PPC32_SYSCALL_DEFINE6	COMPAT_SYSCALL_DEFINE6
> +#endif

I'm fairly sure what you do here is correct, but I am not convinced
we actually need this as long as none of the syscalls take a signed
'long' argument that requires sign-extension for compat mode but
not native 32-bit kernels.

If we add a generic version, it would be nice to always just
use SYSCALL_DEFINEx instead of COMPAT_SYSCALL_DEFINEx. This would
also simplify the syscall table. Do you see a possible problem with
that?

     Arnd

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] powerpc: move sync_file_range2 compat definition
  2022-10-12  3:53 ` [PATCH 2/2] powerpc: move sync_file_range2 compat definition Nicholas Piggin
@ 2022-10-12 10:21   ` Arnd Bergmann
  0 siblings, 0 replies; 7+ messages in thread
From: Arnd Bergmann @ 2022-10-12 10:21 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Jason A . Donenfeld

On Wed, Oct 12, 2022, at 5:53 AM, Nicholas Piggin wrote:
> sync_file_range2 is not a special unaligned-odd-pair calling convention
> syscall, it's just a regular one that does not have a generic compat
> definition. Move it out of sys_ppc32.c and into syscalls.c.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> This one doesn't fix anything and is not required for the previous
> fix, so it could be merged later. Now that we've repurposed sys_ppc32.c
> for the difficult syscalls and compat syscalls live all over the kernel
> now anyway, IMO it's makes things less confusing to move this.

For this one, I would just move the implementation right next to
sync_file_range2() the same way we define compat_sys_sync_file_range(),
and share it with arm64.

      Arnd

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: (subset) [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs
  2022-10-12  3:53 [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs Nicholas Piggin
                   ` (2 preceding siblings ...)
  2022-10-12 10:15 ` Arnd Bergmann
@ 2022-10-14 23:21 ` Michael Ellerman
  2022-10-30 17:03 ` Andreas Schwab
  4 siblings, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2022-10-14 23:21 UTC (permalink / raw)
  To: linuxppc-dev, Nicholas Piggin; +Cc: Jason A . Donenfeld

On Wed, 12 Oct 2022 13:53:34 +1000, Nicholas Piggin wrote:
> powerpc 32-bit system call (and function) calling convention for 64-bit
> arguments requires the next available odd-pair (two sequential registers
> with the first being odd-numbered) from the standard register argument
> allocation.
> 
> The first argument register is r3, so a 64-bit argument that appears at
> an even position in the argument list must skip a register (unless there
> were preceeding 64-bit arguments, which might throw things off). This
> requires non-standard compat definitions to deal with the holes in the
> argument register allocation.
> 
> [...]

Applied to powerpc/fixes.

[1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs
      https://git.kernel.org/powerpc/c/e237506238352f3bfa9cf3983cdab873e35651eb

cheers

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs
  2022-10-12  3:53 [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs Nicholas Piggin
                   ` (3 preceding siblings ...)
  2022-10-14 23:21 ` (subset) " Michael Ellerman
@ 2022-10-30 17:03 ` Andreas Schwab
  4 siblings, 0 replies; 7+ messages in thread
From: Andreas Schwab @ 2022-10-30 17:03 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: Jason A . Donenfeld, linuxppc-dev

On Okt 12 2022, Nicholas Piggin wrote:

> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> index 2bca64f96164..e9e0df4f9a61 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -228,8 +228,10 @@
>  176	64	rt_sigtimedwait			sys_rt_sigtimedwait
>  177	nospu 	rt_sigqueueinfo			sys_rt_sigqueueinfo		compat_sys_rt_sigqueueinfo
>  178	nospu 	rt_sigsuspend			sys_rt_sigsuspend		compat_sys_rt_sigsuspend
> -179	common	pread64				sys_pread64			compat_sys_ppc_pread64
> -180	common	pwrite64			sys_pwrite64			compat_sys_ppc_pwrite64
> +179	32	pread64				sys_ppc_pread64			compat_sys_ppc_pread64
> +179	64	pread64				sys_pread64
> +180	32	pwrite64			sys_ppc_pwrite64		compat_sys_ppc_pwrite64
> +180	64	pwrite64			sys_pwrite64

Doesn't that lack entries for SPU?  Likewise for all other former common
syscalls in this patch.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-10-30 17:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-12  3:53 [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs Nicholas Piggin
2022-10-12  3:53 ` [PATCH 2/2] powerpc: move sync_file_range2 compat definition Nicholas Piggin
2022-10-12 10:21   ` Arnd Bergmann
2022-10-12  7:56 ` [PATCH 1/2] powerpc/32: fix syscall wrappers with 64-bit arguments of unaligned register-pairs Andrew Donnellan
2022-10-12 10:15 ` Arnd Bergmann
2022-10-14 23:21 ` (subset) " Michael Ellerman
2022-10-30 17:03 ` Andreas Schwab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).