diff mbox series

[v3] powerpc/32: remove bogus ppc_select syscall

Message ID f08ef2b6f339ba19987cfef4307a4dd26b2faf97.1614933479.git.christophe.leroy@csgroup.eu (mailing list archive)
State Deferred
Headers show
Series [v3] powerpc/32: remove bogus ppc_select syscall | expand
Related show

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (91966823812efbd175f904599e5cf2a854b39809)
snowpatch_ozlabs/build-ppc64le success Build succeeded
snowpatch_ozlabs/build-ppc64be success Build succeeded
snowpatch_ozlabs/build-ppc64e success Build succeeded
snowpatch_ozlabs/build-pmac32 success Build succeeded
snowpatch_ozlabs/checkpatch warning total: 0 errors, 1 warnings, 0 checks, 50 lines checked
snowpatch_ozlabs/needsstable success Patch has no Fixes tags

Commit Message

Christophe Leroy March 5, 2021, 8:40 a.m. UTC
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.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
[chleroy: Rebased and updated the number of years elapsed and dropped last part of the commit message]
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
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/
A reduced version of it was merged as commit dad2f2fb0fc7 ("powerpc: Fix wrong error code from ppc32 select syscall")

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

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 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(-)

Comments

Arnd Bergmann March 5, 2021, 10:06 a.m. UTC | #1
On Fri, Mar 5, 2021 at 9:40 AM Christophe Leroy
<christophe.leroy@csgroup.eu> 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 description still seems wrong, please drop all the nonsense I wrote
back then and explain what is actually going on.

This is what I can see from the linux-history tree:

- The original ppc32 port (linux-1.3.45) had a regular '__NR_select/sys_select'
  syscall at #82 and an unusable '__NR__newselect/sys_panic' syscall at #142,
  while i386 had the indirect '__NR_select/sys_oldselect' syscall at #82
  and the regular '__NR__newselect/sys_select' version at #142. This was
  rather confusing.

- linux-2.1.48 changed both #82 and #142 to the ppc_select() version that
  tries to guess whether the x86 __NR_select/sys_oldselect() behavior or
  the regular __NR__newselect/sys_select() behavior is used.

- linux-2.5.5 added ppc64 support, with a compat version of ppc_select()
  on both #82 and #142 that would either use the __NR__newselect/sys_select
  semantics or panic() when passed an invalud 'n'. The native ppc64
  port started out with just __NR__newselect/sys_select() on #142

- linux-2.5.19 changed ppc64 compat mode to no longer panic(), making
  both #82 and #142 behave like __NR__newselect/sys_select().

- glibc support for ppc32 gets merged during the linux-2.5 days, supporting
  only #142 with the new behavior.

- linux-2.5.41 dropped support for #82 on ppc64 in compat mode but not
  native ppc32.

- linux-2.6.14 merged the two architecture ports but kept the behavior
  unchanged for both.

- linux-2.6.32 changed the native ppc32 #142 __NR__newselect to
  behave the same as compat mode and no longer emulate the
  x86 oldselect, but #82 remained unchanged.

So we have changed behavior multiple times in the past, and the
current state still theoretically allows running non-glibc binaries that
ran on kernels before 2.1.48 that used either the original powerpc
select or the i386 compatible oldselect semantics. Chances are that
those binaries are broken for some other reason now.

          Arnd
Christophe Leroy March 5, 2021, 10:15 a.m. UTC | #2
Le 05/03/2021 à 11:06, Arnd Bergmann a écrit :
> On Fri, Mar 5, 2021 at 9:40 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> 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 description still seems wrong, please drop all the nonsense I wrote
> back then and explain what is actually going on.
> 
> This is what I can see from the linux-history tree:
> 
> - The original ppc32 port (linux-1.3.45) had a regular '__NR_select/sys_select'
>    syscall at #82 and an unusable '__NR__newselect/sys_panic' syscall at #142,
>    while i386 had the indirect '__NR_select/sys_oldselect' syscall at #82
>    and the regular '__NR__newselect/sys_select' version at #142. This was
>    rather confusing.
> 
> - linux-2.1.48 changed both #82 and #142 to the ppc_select() version that
>    tries to guess whether the x86 __NR_select/sys_oldselect() behavior or
>    the regular __NR__newselect/sys_select() behavior is used.
> 
> - linux-2.5.5 added ppc64 support, with a compat version of ppc_select()
>    on both #82 and #142 that would either use the __NR__newselect/sys_select
>    semantics or panic() when passed an invalud 'n'. The native ppc64
>    port started out with just __NR__newselect/sys_select() on #142
> 
> - linux-2.5.19 changed ppc64 compat mode to no longer panic(), making
>    both #82 and #142 behave like __NR__newselect/sys_select().
> 
> - glibc support for ppc32 gets merged during the linux-2.5 days, supporting
>    only #142 with the new behavior.
> 
> - linux-2.5.41 dropped support for #82 on ppc64 in compat mode but not
>    native ppc32.
> 
> - linux-2.6.14 merged the two architecture ports but kept the behavior
>    unchanged for both.
> 
> - linux-2.6.32 changed the native ppc32 #142 __NR__newselect to
>    behave the same as compat mode and no longer emulate the
>    x86 oldselect, but #82 remained unchanged.
> 
> So we have changed behavior multiple times in the past, and the
> current state still theoretically allows running non-glibc binaries that
> ran on kernels before 2.1.48 that used either the original powerpc
> select or the i386 compatible oldselect semantics. Chances are that
> those binaries are broken for some other reason now.
> 


Whaou, nice archeology, thanks. Do you mind if I copy the history you established ?

In your commit, you said 2.3.48. Here in the history you say 2.1.48. Which one is correct ?

Regardless of whethere binaries are broken or not for other reason, is that worth expecting an 
almost 25 yr old binary to run on future kernels ? If one is able to put the necessary effort to 
port you hardware to the latest kernel, can't he really port the binary as well ?

Thanks
Christophe
Arnd Bergmann March 5, 2021, 12:03 p.m. UTC | #3
On Fri, Mar 5, 2021 at 11:15 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
> Le 05/03/2021 à 11:06, Arnd Bergmann a écrit :
> > On Fri, Mar 5, 2021 at 9:40 AM Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> > - glibc support for ppc32 gets merged during the linux-2.5 days, supporting
> >    only #142 with the new behavior.

It turns out to be older than I said. This was actually in glibc-1.94
from 1997, so during
the linux-2.1 days, not 2.5!

> Whaou, nice archeology, thanks. Do you mind if I copy the history you established ?

That's fine, please copy it.

> In your commit, you said 2.3.48. Here in the history you say 2.1.48. Which one is correct ?

2.1.48 is correct.

> Regardless of whethere binaries are broken or not for other reason, is that worth expecting an
> almost 25 yr old binary to run on future kernels ? If one is able to put the necessary effort to
> port you hardware to the latest kernel, can't he really port the binary as well ?

I think the questions of supporting old hardware with new software and
supporting old
binaries on modern kernels are largely orthogonal. The policy we have
is that we don't
break existing user setups, and it really seems unlikely that anyone
still uses pre-1997
executables for anything that requires a modern kernel!

I now checked the oldest mklinux I could find (DR2.1 from 1997), and
even has the
modern glibc and linux-2.0.28 kernel patched to provide the modern semantics at
syscall #142 for glibc, with the same (already unused) compatibility hack at #82
that we still have for ppc32 today. This made mklinux DR2.1 binaries
incompatible
with mainline linux-2.0 kernels, but they might still work with modern kernels,
regardless of whether we remove support for binaries that worked with mainline
linux-2.0.

       Arnd
Christophe Leroy March 9, 2021, 3:59 p.m. UTC | #4
Le 05/03/2021 à 13:03, Arnd Bergmann a écrit :
> On Fri, Mar 5, 2021 at 11:15 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>> Le 05/03/2021 à 11:06, Arnd Bergmann a écrit :
>>> On Fri, Mar 5, 2021 at 9:40 AM Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>> - glibc support for ppc32 gets merged during the linux-2.5 days, supporting
>>>     only #142 with the new behavior.
> 
> It turns out to be older than I said. This was actually in glibc-1.94
> from 1997, so during
> the linux-2.1 days, not 2.5!
> 
>> Whaou, nice archeology, thanks. Do you mind if I copy the history you established ?
> 
> That's fine, please copy it.
> 
>> In your commit, you said 2.3.48. Here in the history you say 2.1.48. Which one is correct ?
> 
> 2.1.48 is correct.
> 
>> Regardless of whethere binaries are broken or not for other reason, is that worth expecting an
>> almost 25 yr old binary to run on future kernels ? If one is able to put the necessary effort to
>> port you hardware to the latest kernel, can't he really port the binary as well ?
> 
> I think the questions of supporting old hardware with new software and
> supporting old
> binaries on modern kernels are largely orthogonal. The policy we have
> is that we don't
> break existing user setups, and it really seems unlikely that anyone
> still uses pre-1997
> executables for anything that requires a modern kernel!
> 
> I now checked the oldest mklinux I could find (DR2.1 from 1997), and
> even has the
> modern glibc and linux-2.0.28 kernel patched to provide the modern semantics at
> syscall #142 for glibc, with the same (already unused) compatibility hack at #82
> that we still have for ppc32 today. This made mklinux DR2.1 binaries
> incompatible
> with mainline linux-2.0 kernels, but they might still work with modern kernels,
> regardless of whether we remove support for binaries that worked with mainline
> linux-2.0.


I had another look. In fact x86, arm and m68k still have the #82 syscall, but they don't have the 
hack we have on powerpc to "guess" that something is calling the old select with the arguments of 
the new select.

As part of my series of user accesses cleanup, I'll replace the open coded stuff by a call to 
sys_old_select(), see below.

Maybe at the end we should keep the #82 syscall, but do we need to keep the powerpc hack really ? 
Maybe the best is to drop ppc_select() function but mention sys_old_select() instead of ni_syscall 
for entry #82 in the syscall table ?

Christophe
---
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index 700fcdac2e3c..b541c690a31c 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -40,6 +40,7 @@
  #define __ARCH_WANT_SYS_SIGPROCMASK
  #ifdef CONFIG_PPC32
  #define __ARCH_WANT_OLD_STAT
+#define __ARCH_WANT_SYS_OLD_SELECT
  #endif
  #ifdef CONFIG_PPC64
  #define __ARCH_WANT_SYS_TIME
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 078608ec2e92..a552c9e68d7e 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -82,16 +82,8 @@ 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_old_select((void __user *)n);
+
  	return sys_select(n, inp, outp, exp, tvp);
  }
  #endif
Arnd Bergmann March 10, 2021, 10 a.m. UTC | #5
On Tue, Mar 9, 2021 at 4:59 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
> Le 05/03/2021 à 13:03, Arnd Bergmann a écrit :
> > On Fri, Mar 5, 2021 at 11:15 AM Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >> Le 05/03/2021 à 11:06, Arnd Bergmann a écrit :
>
> I had another look. In fact x86, arm and m68k still have the #82 syscall, but they don't have the
> hack we have on powerpc to "guess" that something is calling the old select with the arguments of
> the new select.

Makes sense. At least for x86, there are probably still some pre-glibc
binaries around
that would use the old select.

> As part of my series of user accesses cleanup, I'll replace the open coded stuff by a call to
> sys_old_select(), see below.

Nice!

> Maybe at the end we should keep the #82 syscall, but do we need to keep the powerpc hack really ?
> Maybe the best is to drop ppc_select() function but mention sys_old_select() instead of ni_syscall
> for entry #82 in the syscall table ?

I'd say we should either keep the powerpc hack intact (with your cleanup),
or remove the syscall entirely. I have been unable to find any indication of
who might have called the #82 syscall prior to mklinux DR2.1 and linuxppc R4,
so there is no way of knowing which of the two ABIs they were using either.

It was definitely ambiguous since the ancient kernels only had sys_select()
semantics at #82, but the existence of the hack tells us that there were at
least some binaries that wanted the sys_oldselect() semantics.

       Arnd
diff mbox series

Patch

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