[PATCH v2] powerpc/32: remove bogus ppc_select syscall

From: Arnd Bergmann <arnd@arndb.de>

The ppc_select function was introduced in linux-2.3.48 in order to support
code confusing the legacy select() calling convention with the standard one.
Even 24 years ago, all correctly built code should not have done this and
could have easily been phased out. Nothing that was compiled later should
actually try to use the old_select interface, and it would have been broken
already on all ppc64 kernels with the syscall emulation layer.

This patch brings the 32 bit compat ABI and the native 32 bit ABI for
powerpc into a consistent state, by removing support for both the
old_select system call number and the handler for it.

The bug report triggering this came from
Halesh Sadashiiv [off-list ref], who discovered that the
32 bit implementation of ppc_select would in case of a negative number
of file descriptors incorrectly return -EFAULT instead of -EINVAL.
There seems to be no way to fix this problem in a way that would
keep broken pre-1997 binaries running.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Halesh Sadashiiv <redacted>
[chleroy: Rebased and updated the number of years elapsed in the commit message]
Signed-off-by: Christophe Leroy <redacted>
---
First version was in 2008, at that time it was rejected, see
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/200809240839.14902.arnd@arndb.de/

If we decide to still keep this, then we'll have to:
- take into account -4096 < fd < 0 case
- use unsafe_get_user inside a uaccess_begin block
---
 arch/powerpc/include/asm/asm-prototypes.h |  3 ---
 arch/powerpc/kernel/syscalls.c            | 25 -----------------------
 arch/powerpc/kernel/syscalls/syscall.tbl  |  4 +---
 3 files changed, 1 insertion(+), 31 deletions(-)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 939f3c94c8f3..78e0a3bd448a 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h

@@ -63,9 +63,6 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
 #ifdef CONFIG_PPC32
 long sys_debug_setcontext(struct ucontext __user *ctx,
 			  int ndbg, struct sig_dbg_op __user *dbg);
-int
-ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp,
-	   struct __kernel_old_timeval __user *tvp);
 unsigned long __init early_init(unsigned long dt_ptr);
 void __init machine_init(u64 dt_ptr);
 #endif

diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 078608ec2e92..70b0eb5bedfd 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c

@@ -71,31 +71,6 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
 	return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
 }
 
-#ifdef CONFIG_PPC32
-/*
- * Due to some executables calling the wrong select we sometimes
- * get wrong args.  This determines how the args are being passed
- * (a single ptr to them all args passed) then calls
- * sys_select() with the appropriate args. -- Cort
- */
-int
-ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct __kernel_old_timeval __user *tvp)
-{
-	if ( (unsigned long)n >= 4096 )
-	{
-		unsigned long __user *buffer = (unsigned long __user *)n;
-		if (!access_ok(buffer, 5*sizeof(unsigned long))
-		    || __get_user(n, buffer)
-		    || __get_user(inp, ((fd_set __user * __user *)(buffer+1)))
-		    || __get_user(outp, ((fd_set  __user * __user *)(buffer+2)))
-		    || __get_user(exp, ((fd_set  __user * __user *)(buffer+3)))
-		    || __get_user(tvp, ((struct __kernel_old_timeval  __user * __user *)(buffer+4))))
-			return -EFAULT;
-	}
-	return sys_select(n, inp, outp, exp, tvp);
-}
-#endif
-
 #ifdef CONFIG_PPC64
 long ppc64_personality(unsigned long personality)
 {

diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 0b2480cf3e47..5bb0e90e502e 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl

@@ -110,9 +110,7 @@
 79	common	settimeofday			sys_settimeofday		compat_sys_settimeofday
 80	common	getgroups			sys_getgroups
 81	common	setgroups			sys_setgroups
-82	32	select				ppc_select			sys_ni_syscall
-82	64	select				sys_ni_syscall
-82	spu	select				sys_ni_syscall
+82	common	select				sys_ni_syscall
 83	common	symlink				sys_symlink
 84	32	oldlstat			sys_lstat			sys_ni_syscall
 84	64	oldlstat			sys_ni_syscall

-- 
2.25.0

On Thu, Mar 4, 2021 at 1:51 PM Christophe Leroy
[off-list ref] wrote:

From: Arnd Bergmann <arnd@arndb.de>

The ppc_select function was introduced in linux-2.3.48 in order to support
code confusing the legacy select() calling convention with the standard one.
Even 24 years ago, all correctly built code should not have done this and
could have easily been phased out. Nothing that was compiled later should
actually try to use the old_select interface, and it would have been broken
already on all ppc64 kernels with the syscall emulation layer.

This patch brings the 32 bit compat ABI and the native 32 bit ABI for
powerpc into a consistent state, by removing support for both the
old_select system call number and the handler for it.

The bug report triggering this came from
Halesh Sadashiiv [off-list ref], who discovered that the
32 bit implementation of ppc_select would in case of a negative number
of file descriptors incorrectly return -EFAULT instead of -EINVAL.
There seems to be no way to fix this problem in a way that would
keep broken pre-1997 binaries running.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Halesh Sadashiiv <redacted>
[chleroy: Rebased and updated the number of years elapsed in the commit message]
Signed-off-by: Christophe Leroy <redacted>
---
First version was in 2008, at that time it was rejected, see
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/200809240839.14902.arnd@arndb.de/

The patch from 2008 did two things:

- it removed the ppc32 specific 'select' syscall at #82
- it fixed the generic '_newselect' syscall at #142

Back then, the decision was to only address the second issue, which
got merged in commit dad2f2fb0fc7 ("powerpc: Fix wrong error code from
ppc32 select syscall").

It is probably ok to remove the old select system call now, but
my changelog text no longer makes sense, as the patch has nothing
to do with the bug that was reported back then.

       Arnd

Le 04/03/2021 à 16:17, Arnd Bergmann a écrit :

On Thu, Mar 4, 2021 at 1:51 PM Christophe Leroy
[off-list ref] wrote:

quoted

From: Arnd Bergmann <arnd@arndb.de>

The ppc_select function was introduced in linux-2.3.48 in order to support
code confusing the legacy select() calling convention with the standard one.
Even 24 years ago, all correctly built code should not have done this and
could have easily been phased out. Nothing that was compiled later should
actually try to use the old_select interface, and it would have been broken
already on all ppc64 kernels with the syscall emulation layer.

This patch brings the 32 bit compat ABI and the native 32 bit ABI for
powerpc into a consistent state, by removing support for both the
old_select system call number and the handler for it.

The bug report triggering this came from
Halesh Sadashiiv [off-list ref], who discovered that the
32 bit implementation of ppc_select would in case of a negative number
of file descriptors incorrectly return -EFAULT instead of -EINVAL.
There seems to be no way to fix this problem in a way that would
keep broken pre-1997 binaries running.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Halesh Sadashiiv <redacted>
[chleroy: Rebased and updated the number of years elapsed in the commit message]
Signed-off-by: Christophe Leroy <redacted>
---
First version was in 2008, at that time it was rejected, see
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/200809240839.14902.arnd@arndb.de/

The patch from 2008 did two things:

- it removed the ppc32 specific 'select' syscall at #82
- it fixed the generic '_newselect' syscall at #142

Back then, the decision was to only address the second issue, which
got merged in commit dad2f2fb0fc7 ("powerpc: Fix wrong error code from
ppc32 select syscall").

It is probably ok to remove the old select system call now, but
my changelog text no longer makes sense, as the patch has nothing
to do with the bug that was reported back then.

I understood that the original reported bug was that calling that version of select() with a 
negative value as first parametre would lead to a -EFAULT instead of -EINVAL. That's exactly the 
case here, if you set n = -1 you get into this (unsigned long)n > 4096 then the buffer is at 
0xffffffff and access_ok() won't grand access to it so the return value will be -EFAULT instead of 
-EINVAL.

Am I missing something ?

Christophe

On Thu, Mar 4, 2021 at 4:24 PM Christophe Leroy
[off-list ref] wrote:

Le 04/03/2021 à 16:17, Arnd Bergmann a écrit :

quoted

On Thu, Mar 4, 2021 at 1:51 PM Christophe Leroy [off-list ref] wrote:

quoted

---
First version was in 2008, at that time it was rejected, see
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/200809240839.14902.arnd@arndb.de/

The patch from 2008 did two things:

- it removed the ppc32 specific 'select' syscall at #82
- it fixed the generic '_newselect' syscall at #142

Back then, the decision was to only address the second issue, which
got merged in commit dad2f2fb0fc7 ("powerpc: Fix wrong error code from
ppc32 select syscall").

It is probably ok to remove the old select system call now, but
my changelog text no longer makes sense, as the patch has nothing
to do with the bug that was reported back then.

I understood that the original reported bug was that calling that version of select() with a
negative value as first parametre would lead to a -EFAULT instead of -EINVAL. That's exactly the
case here, if you set n = -1 you get into this (unsigned long)n > 4096 then the buffer is at
0xffffffff and access_ok() won't grand access to it so the return value will be -EFAULT instead of
-EINVAL.

Am I missing something ?

This is the behavior of the ppc_select() implementation, but as far as
I can tell,
the bug report was for the problem that this behavior would happen for both
syscall #82 and syscall #142 when the correct behavior would have been to
only do it for #82 but not for #142.

       Arnd