linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* RE: Regarding select() on PPC
@ 2008-09-23  7:16 Sadashiiv, Halesh
  2008-09-23  9:07 ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 24+ messages in thread
From: Sadashiiv, Halesh @ 2008-09-23  7:16 UTC (permalink / raw)
  To: benh, Arnd Bergmann; +Cc: linuxppc-embedded



>-----Original Message-----
>From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
>Sent: Tuesday, September 23, 2008 12:00 PM
>To: Arnd Bergmann
>Cc: linuxppc-embedded@ozlabs.org; Sadashiiv, Halesh
>Subject: Re: Regarding select() on PPC
>
>On Tue, 2008-09-23 at 08:01 +0200, Arnd Bergmann wrote:
>> No, sorry, I don't have a git history that I can search for that.
>> I have a strong guess that it would be the end of the last century
>> though, probably before 2.3.48, since the comment in there already
>> refers to legacy binaries.
>
>Yeah, I'm keen to rip it out.

Means argument checking in ppc_select() can be removed??
Than shall I post patch to linuxppc-dev@ozlabs.org

>
>Ben.
>
>



-------------------------------------------------------------------
This email is confidential and intended only for the use of the =
individual or entity named above and may contain information that is =
privileged. If you are not the intended recipient, you are notified that =
any dissemination, distribution or copying of this email is strictly =
prohibited. If you have received this email in error, please notify us =
immediately by return email or telephone and destroy the original =
message. - This mail is sent via Sony Asia Pacific Mail Gateway.
-------------------------------------------------------------------

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

* RE: Regarding select() on PPC
  2008-09-23  7:16 Regarding select() on PPC Sadashiiv, Halesh
@ 2008-09-23  9:07 ` Benjamin Herrenschmidt
  2008-09-24  6:23   ` Arnd Bergmann
  0 siblings, 1 reply; 24+ messages in thread
From: Benjamin Herrenschmidt @ 2008-09-23  9:07 UTC (permalink / raw)
  To: Sadashiiv, Halesh; +Cc: Arnd Bergmann, linuxppc-embedded

On Tue, 2008-09-23 at 12:46 +0530, Sadashiiv, Halesh wrote:
> 
> >-----Original Message-----
> >From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
> >Sent: Tuesday, September 23, 2008 12:00 PM
> >To: Arnd Bergmann
> >Cc: linuxppc-embedded@ozlabs.org; Sadashiiv, Halesh
> >Subject: Re: Regarding select() on PPC
> >
> >On Tue, 2008-09-23 at 08:01 +0200, Arnd Bergmann wrote:
> >> No, sorry, I don't have a git history that I can search for that.
> >> I have a strong guess that it would be the end of the last century
> >> though, probably before 2.3.48, since the comment in there already
> >> refers to legacy binaries.
> >
> >Yeah, I'm keen to rip it out.
> 
> Means argument checking in ppc_select() can be removed??
> Than shall I post patch to linuxppc-dev@ozlabs.org

ppc_select can probably be removed alltogether and ppc32_select too

Ben.

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

* Re: Regarding select() on PPC
  2008-09-23  9:07 ` Benjamin Herrenschmidt
@ 2008-09-24  6:23   ` Arnd Bergmann
  2008-09-24  6:28     ` Benjamin Herrenschmidt
  2008-09-24  6:39     ` [PATCH] remove bogus ppc_select syscall Arnd Bergmann
  0 siblings, 2 replies; 24+ messages in thread
From: Arnd Bergmann @ 2008-09-24  6:23 UTC (permalink / raw)
  To: benh; +Cc: Sadashiiv, Halesh, linuxppc-embedded

On Tuesday 23 September 2008, Benjamin Herrenschmidt wrote:
> 
> > Means argument checking in ppc_select() can be removed??
> > Than shall I post patch to linuxppc-dev@ozlabs.org
> 
> ppc_select can probably be removed alltogether and ppc32_select too

I think ppc32_select needs to stay. It never did the hack for
supporting the old_select calling conventions, but it does the
sign extension for the 32 bit arguments, which I think you still
want.

In any way, both the 32 bit emulation and the native ppc32 code
path should do the same thing, which currently they don't.

	Arnd <><

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

* Re: Regarding select() on PPC
  2008-09-24  6:23   ` Arnd Bergmann
@ 2008-09-24  6:28     ` Benjamin Herrenschmidt
  2008-09-24  6:45       ` Arnd Bergmann
  2008-09-24  6:39     ` [PATCH] remove bogus ppc_select syscall Arnd Bergmann
  1 sibling, 1 reply; 24+ messages in thread
From: Benjamin Herrenschmidt @ 2008-09-24  6:28 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Sadashiiv, Halesh, linuxppc-embedded

On Wed, 2008-09-24 at 08:23 +0200, Arnd Bergmann wrote:
> On Tuesday 23 September 2008, Benjamin Herrenschmidt wrote:
> > 
> > > Means argument checking in ppc_select() can be removed??
> > > Than shall I post patch to linuxppc-dev@ozlabs.org
> > 
> > ppc_select can probably be removed alltogether and ppc32_select too
> 
> I think ppc32_select needs to stay. It never did the hack for
> supporting the old_select calling conventions, but it does the
> sign extension for the 32 bit arguments, which I think you still
> want.

Why do we want it ?

It's an int, it's zero extended, but that should still be a valid signed
32 bits int in compat_sys_select() or do I miss something ?
 
Ben.

> In any way, both the 32 bit emulation and the native ppc32 code
> path should do the same thing, which currently they don't.
> 
> 	Arnd <><

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

* [PATCH] remove bogus ppc_select syscall
  2008-09-24  6:23   ` Arnd Bergmann
  2008-09-24  6:28     ` Benjamin Herrenschmidt
@ 2008-09-24  6:39     ` Arnd Bergmann
  2008-09-24 16:29       ` Paul Mackerras
  2008-10-10  4:29       ` Benjamin Herrenschmidt
  1 sibling, 2 replies; 24+ messages in thread
From: Arnd Bergmann @ 2008-09-24  6:39 UTC (permalink / raw)
  To: linuxppc-embedded, paulus; +Cc: Sadashiiv, Halesh, linuxppc-dev

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 11 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 <halesh.sadashiv@ap.sony.com>, 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 <halesh.sadashiv@ap.sony.com>
---

Halesh, please test this patch to make sure it fixes the problem
you reported. I do not have a ppc32 machine I can try this on.

diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index e07d0c7..46107cc 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -92,7 +92,7 @@
 #define __NR_settimeofday       79
 #define __NR_getgroups          80
 #define __NR_setgroups          81
-#define __NR_select             82
+/* Number 82 was the old (pre-1.3.x) select */
 #define __NR_symlink            83
 #define __NR_oldlstat           84
 #define __NR_readlink           85
diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index f6cc7a4..5a69b32 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -85,7 +85,7 @@ COMPAT_SYS_SPU(gettimeofday)
 COMPAT_SYS_SPU(settimeofday)
 COMPAT_SYS_SPU(getgroups)
 COMPAT_SYS_SPU(setgroups)
-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
+SYSCALL(sys_ni_syscall)
 SYSCALL_SPU(symlink)
 OLDSYS(lstat)
 COMPAT_SYS_SPU(readlink)
@@ -145,7 +145,7 @@ SYSCALL_SPU(setfsuid)
 SYSCALL_SPU(setfsgid)
 SYSCALL_SPU(llseek)
 COMPAT_SYS_SPU(getdents)
-SYSX_SPU(sys_select,ppc32_select,ppc_select)
+SYSX_SPU(sys_select,ppc32_select,sys_select)
 SYSCALL_SPU(flock)
 SYSCALL_SPU(msync)
 COMPAT_SYS_SPU(readv)
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index c04832c..c2e6a74 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -183,31 +183,6 @@ unsigned long sys_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 timeval __user *tvp)
-{
-	if ( (unsigned long)n >= 4096 )
-	{
-		unsigned long __user *buffer = (unsigned long __user *)n;
-		if (!access_ok(VERIFY_READ, 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 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)
 {

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

* Re: Regarding select() on PPC
  2008-09-24  6:28     ` Benjamin Herrenschmidt
@ 2008-09-24  6:45       ` Arnd Bergmann
  2008-09-24  7:08         ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2008-09-24  6:45 UTC (permalink / raw)
  To: linuxppc-embedded, benh; +Cc: Sadashiiv, Halesh

On Wednesday 24 September 2008, Benjamin Herrenschmidt wrote:
> Why do we want it ?
> 
> It's an int, it's zero extended, but that should still be a valid signed
> 32 bits int in compat_sys_select() or do I miss something ?

IIRC, the calling conventions on 64 bit ppc assume that a signed int
argument is sign-extended into a 64 bit register. The compat syscall
entry point does not know which registers are signed or unsigned, so
it will always to zero-extend, making the register contain an undefined
bit pattern (e.g. 0x00000000fffffffe), which may be interpreted as being
a positive number. An explicit cast as it is done in ppc32_select turns
this into a well-defined 32-bit number stored in a 64-bit register (e.g.
0xfffffffffffffffe).

	Arnd <><

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

* Re: Regarding select() on PPC
  2008-09-24  6:45       ` Arnd Bergmann
@ 2008-09-24  7:08         ` Benjamin Herrenschmidt
  2008-09-24 21:46           ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 24+ messages in thread
From: Benjamin Herrenschmidt @ 2008-09-24  7:08 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Sadashiiv, Halesh, linuxppc-embedded

On Wed, 2008-09-24 at 08:45 +0200, Arnd Bergmann wrote:
> On Wednesday 24 September 2008, Benjamin Herrenschmidt wrote:
> > Why do we want it ?
> > 
> > It's an int, it's zero extended, but that should still be a valid signed
> > 32 bits int in compat_sys_select() or do I miss something ?
> 
> IIRC, the calling conventions on 64 bit ppc assume that a signed int
> argument is sign-extended into a 64 bit register. The compat syscall
> entry point does not know which registers are signed or unsigned, so
> it will always to zero-extend, making the register contain an undefined
> bit pattern (e.g. 0x00000000fffffffe), which may be interpreted as being
> a positive number. An explicit cast as it is done in ppc32_select turns
> this into a well-defined 32-bit number stored in a 64-bit register (e.g.
> 0xfffffffffffffffe).

If that is true, then _any_ syscall that takes an int value must have
a ppc specific sign-extending compat wrapper, are you sure that is
the case ?

Ben.

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

* Re: [PATCH] remove bogus ppc_select syscall
  2008-09-24  6:39     ` [PATCH] remove bogus ppc_select syscall Arnd Bergmann
@ 2008-09-24 16:29       ` Paul Mackerras
  2008-09-24 17:03         ` Arnd Bergmann
  2008-10-10  4:29       ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 24+ messages in thread
From: Paul Mackerras @ 2008-09-24 16:29 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Sadashiiv, Halesh, linuxppc-embedded

Arnd Bergmann writes:

> diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
> index f6cc7a4..5a69b32 100644
> --- a/arch/powerpc/include/asm/systbl.h
> +++ b/arch/powerpc/include/asm/systbl.h
> @@ -85,7 +85,7 @@ COMPAT_SYS_SPU(gettimeofday)
>  COMPAT_SYS_SPU(settimeofday)
>  COMPAT_SYS_SPU(getgroups)
>  COMPAT_SYS_SPU(setgroups)
> -SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
> +SYSCALL(sys_ni_syscall)

I don't see any reason to remove the old select syscall on 32-bit
kernels.  I think this hunk below is the only part of the patch that
we actually need:

> @@ -145,7 +145,7 @@ SYSCALL_SPU(setfsuid)
>  SYSCALL_SPU(setfsgid)
>  SYSCALL_SPU(llseek)
>  COMPAT_SYS_SPU(getdents)
> -SYSX_SPU(sys_select,ppc32_select,ppc_select)
> +SYSX_SPU(sys_select,ppc32_select,sys_select)

Paul.

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

* Re: [PATCH] remove bogus ppc_select syscall
  2008-09-24 16:29       ` Paul Mackerras
@ 2008-09-24 17:03         ` Arnd Bergmann
  2008-10-10  7:40           ` Paul Mackerras
  0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2008-09-24 17:03 UTC (permalink / raw)
  To: linuxppc-embedded; +Cc: linuxppc-dev, Sadashiiv, Halesh, Paul Mackerras

On Wednesday 24 September 2008, Paul Mackerras wrote:
> > diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/a=
sm/systbl.h
> > index f6cc7a4..5a69b32 100644
> > --- a/arch/powerpc/include/asm/systbl.h
> > +++ b/arch/powerpc/include/asm/systbl.h
> > @@ -85,7 +85,7 @@ COMPAT_SYS_SPU(gettimeofday)
> > =A0COMPAT_SYS_SPU(settimeofday)
> > =A0COMPAT_SYS_SPU(getgroups)
> > =A0COMPAT_SYS_SPU(setgroups)
> > -SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
> > +SYSCALL(sys_ni_syscall)
>=20
> I don't see any reason to remove the old select syscall on 32-bit
> kernels. =A0

Well, the point I made earlier ist that the native ppc32 path should
behave the same way as the compat ppc32 path. If we keep ppc_select
in one way or another, we should also have a compat wrapper for that,
right?

	Arnd <><

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

* Re: Regarding select() on PPC
  2008-09-24  7:08         ` Benjamin Herrenschmidt
@ 2008-09-24 21:46           ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 24+ messages in thread
From: Benjamin Herrenschmidt @ 2008-09-24 21:46 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Sadashiiv, Halesh, linuxppc-embedded


> If that is true, then _any_ syscall that takes an int value must have
> a ppc specific sign-extending compat wrapper, are you sure that is
> the case ?

Ok, so that's crap that has been bitrotting. We -do- need to sign extend
everywhere and it looks like we forgot to do it on anything recent, like
signalfd4...

Ben.

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

* Re: [PATCH] remove bogus ppc_select syscall
  2008-09-24  6:39     ` [PATCH] remove bogus ppc_select syscall Arnd Bergmann
  2008-09-24 16:29       ` Paul Mackerras
@ 2008-10-10  4:29       ` Benjamin Herrenschmidt
  2008-10-10  7:43         ` Paul Mackerras
  1 sibling, 1 reply; 24+ messages in thread
From: Benjamin Herrenschmidt @ 2008-10-10  4:29 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Sadashiiv, Halesh, paulus, linuxppc-embedded

On Wed, 2008-09-24 at 08:39 +0200, Arnd Bergmann wrote:
> 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 11 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.

 .../...

It's me or the patch is whitespaces damaged ?

Cheers,
Ben.

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

* Re: [PATCH] remove bogus ppc_select syscall
  2008-09-24 17:03         ` Arnd Bergmann
@ 2008-10-10  7:40           ` Paul Mackerras
  2008-10-10  8:44             ` Arnd Bergmann
  0 siblings, 1 reply; 24+ messages in thread
From: Paul Mackerras @ 2008-10-10  7:40 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Sadashiiv, Halesh, linuxppc-embedded

Arnd Bergmann writes:

> Well, the point I made earlier ist that the native ppc32 path should
> behave the same way as the compat ppc32 path. If we keep ppc_select
> in one way or another, we should also have a compat wrapper for that,
> right?

No - we have other old system calls that don't exist at all on a
64-bit kernel but do on a 32-bit kernel.  We decided a long time ago
that there was no point supporting really ancient 32-bit userland code
on a 64-bit kernel.  That doesn't mean we need to rip out the support
from the 32-bit kernel.

Paul.

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

* Re: [PATCH] remove bogus ppc_select syscall
  2008-10-10  4:29       ` Benjamin Herrenschmidt
@ 2008-10-10  7:43         ` Paul Mackerras
  2008-10-10  8:45           ` Arnd Bergmann
  2008-10-10 23:17           ` Benjamin Herrenschmidt
  0 siblings, 2 replies; 24+ messages in thread
From: Paul Mackerras @ 2008-10-10  7:43 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev, Sadashiiv, Halesh, Arnd Bergmann, linuxppc-embedded

Benjamin Herrenschmidt writes:

> On Wed, 2008-09-24 at 08:39 +0200, Arnd Bergmann wrote:
> > 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 11 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.
> 
>  .../...
> 
> It's me or the patch is whitespaces damaged ?

Please don't apply Arnd's patch.  As I said, all we need is this
one-line change in arch/powerpc/include/asm/systbl.h:

-SYSX_SPU(sys_select,ppc32_select,ppc_select)
+SYSX_SPU(sys_select,ppc32_select,sys_select)

Paul.

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

* Re: [PATCH] remove bogus ppc_select syscall
  2008-10-10  7:40           ` Paul Mackerras
@ 2008-10-10  8:44             ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2008-10-10  8:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sadashiiv, Halesh, Paul Mackerras, linuxppc-embedded

On Friday 10 October 2008, Paul Mackerras wrote:
> Arnd Bergmann writes:
>=20
> > Well, the point I made earlier ist that the native ppc32 path should
> > behave the same way as the compat ppc32 path. If we keep ppc_select
> > in one way or another, we should also have a compat wrapper for that,
> > right?
>=20
> No - we have other old system calls that don't exist at all on a
> 64-bit kernel but do on a 32-bit kernel. =A0We decided a long time ago
> that there was no point supporting really ancient 32-bit userland code
> on a 64-bit kernel. =A0That doesn't mean we need to rip out the support
> from the 32-bit kernel.

Ok, that makes sense, thanks for the clarification.

	Arnd <><

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

* Re: [PATCH] remove bogus ppc_select syscall
  2008-10-10  7:43         ` Paul Mackerras
@ 2008-10-10  8:45           ` Arnd Bergmann
  2008-10-10 23:17           ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2008-10-10  8:45 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sadashiiv, Halesh, Paul Mackerras, linuxppc-embedded

On Friday 10 October 2008, Paul Mackerras wrote:
> Please don't apply Arnd's patch. =A0As I said, all we need is this
> one-line change in arch/powerpc/include/asm/systbl.h:
>=20
> -SYSX_SPU(sys_select,ppc32_select,ppc_select)
> +SYSX_SPU(sys_select,ppc32_select,sys_select)
>=20
> Paul.

=46WIW,

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

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

* Re: [PATCH] remove bogus ppc_select syscall
  2008-10-10  7:43         ` Paul Mackerras
  2008-10-10  8:45           ` Arnd Bergmann
@ 2008-10-10 23:17           ` Benjamin Herrenschmidt
  2008-10-12  8:58             ` Arnd Bergmann
  2008-10-13  2:42             ` i2o driver work with AMCC 460EX? Ayman El-Khashab
  1 sibling, 2 replies; 24+ messages in thread
From: Benjamin Herrenschmidt @ 2008-10-10 23:17 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: linuxppc-dev, Sadashiiv, Halesh, Arnd Bergmann, linuxppc-embedded

On Fri, 2008-10-10 at 18:43 +1100, Paul Mackerras wrote:
> > It's me or the patch is whitespaces damaged ?
> 
> Please don't apply Arnd's patch.  As I said, all we need is this
> one-line change in arch/powerpc/include/asm/systbl.h:
> 
> -SYSX_SPU(sys_select,ppc32_select,ppc_select)
> +SYSX_SPU(sys_select,ppc32_select,sys_select)

Ok, so you want to keep compat with the old stuff. Note that we still
have this weird thing that on 64-bit kernels, we don't provide this.

Ben.

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

* Re: [PATCH] remove bogus ppc_select syscall
  2008-10-10 23:17           ` Benjamin Herrenschmidt
@ 2008-10-12  8:58             ` Arnd Bergmann
  2008-10-13  2:42             ` i2o driver work with AMCC 460EX? Ayman El-Khashab
  1 sibling, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2008-10-12  8:58 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev, Sadashiiv, Halesh, Paul Mackerras, linuxppc-embedded

On Saturday 11 October 2008, Benjamin Herrenschmidt wrote:
> > 
> > -SYSX_SPU(sys_select,ppc32_select,ppc_select)
> > +SYSX_SPU(sys_select,ppc32_select,sys_select)
> 
> Ok, so you want to keep compat with the old stuff. Note that we still
> have this weird thing that on 64-bit kernels, we don't provide this.

This change makes the funny wrapper only active for the "old" select
number on native ppc32, which is not active on ppc64 or compat32,
while any programs using the "new" select number, i.e. any binay you
will ever encounter in practice, now gets the standard semantics.

	Arnd <><

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

* i2o driver work with AMCC 460EX?
  2008-10-10 23:17           ` Benjamin Herrenschmidt
  2008-10-12  8:58             ` Arnd Bergmann
@ 2008-10-13  2:42             ` Ayman El-Khashab
  1 sibling, 0 replies; 24+ messages in thread
From: Ayman El-Khashab @ 2008-10-13  2:42 UTC (permalink / raw)
  To: linuxppc-embedded


Does the linux i2o driver / subsystem work with the i2o unit
in the AMCC 460EX?  In the AMCC part it is part of the PCI-E
interface. =20

Thanks
ayman

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

* RE: [PATCH] remove bogus ppc_select syscall
  2008-10-13 11:59 Sadashiiv, Halesh
@ 2008-10-13 21:40 ` Paul Mackerras
  0 siblings, 0 replies; 24+ messages in thread
From: Paul Mackerras @ 2008-10-13 21:40 UTC (permalink / raw)
  To: Sadashiiv, Halesh; +Cc: linuxppc-dev, Arnd Bergmann, linuxppc-embedded

Sadashiiv, Halesh writes:

> I have tested the testcase that I have provided at the time of reporting
> this issue. But it didn't work as expected with the above patch you
> provided.
> (Failed to return EINVAL on negative value of n to select())

How is your testcase invoking select?  Is it doing its own hand-coded
system call using __NR_select?  If so the testcase is broken - it
should be using __NR__newselect.

If the test is using the ordinary glibc select(), then please send the
source code of the test program.

Paul.

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

* RE: [PATCH] remove bogus ppc_select syscall
@ 2008-10-13 12:18 Sadashiiv, Halesh
  0 siblings, 0 replies; 24+ messages in thread
From: Sadashiiv, Halesh @ 2008-10-13 12:18 UTC (permalink / raw)
  To: Sadashiiv, Halesh, Paul Mackerras, benh
  Cc: linuxppc-dev, Arnd Bergmann, linuxppc-embedded




>>Please don't apply Arnd's patch.  As I said, all we need is this
>>one-line change in arch/powerpc/include/asm/systbl.h:
>>
>>-SYSX_SPU(sys_select,ppc32_select,ppc_select)
>>+SYSX_SPU(sys_select,ppc32_select,sys_select)
>
>
>I have tested the testcase that I have provided at the time of
reporting
>this issue. But it didn't work as expected with the above patch you
>provided.
>(Failed to return EINVAL on negative value of n to select())
>
>Please let me know about this. I have tested on PPC32 2.6.23.

Hi all,

Apologies, Test failed because of small typo in test case, The above
patch provided has been tested on PPC32. Works fine and as expected.

Reported-by: Halesh Sadashiv <halesh.sadashiv@ap.sony.com>
Tested-by: Halesh Sadashiv <halesh.sadashiv@ap.sony.com>

Thanks,
Halesh


>
>-Halesh
>
>>
>>Paul.



-------------------------------------------------------------------
This email is confidential and intended only for the use of the =
individual or entity named above and may contain information that is =
privileged. If you are not the intended recipient, you are notified that =
any dissemination, distribution or copying of this email is strictly =
prohibited. If you have received this email in error, please notify us =
immediately by return email or telephone and destroy the original =
message. - This mail is sent via Sony Asia Pacific Mail Gateway.
-------------------------------------------------------------------

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

* RE: [PATCH] remove bogus ppc_select syscall
@ 2008-10-13 11:59 Sadashiiv, Halesh
  2008-10-13 21:40 ` Paul Mackerras
  0 siblings, 1 reply; 24+ messages in thread
From: Sadashiiv, Halesh @ 2008-10-13 11:59 UTC (permalink / raw)
  To: Paul Mackerras, benh; +Cc: linuxppc-dev, Arnd Bergmann, linuxppc-embedded



>-----Original Message-----
>From: Paul Mackerras [mailto:paulus@samba.org]
>Sent: Friday, October 10, 2008 1:14 PM
>To: benh@kernel.crashing.org
>Cc: Arnd Bergmann; linuxppc-embedded@ozlabs.org; Sadashiiv, Halesh;
>linuxppc-dev@ozlabs.org
>Subject: Re: [PATCH] remove bogus ppc_select syscall
>
>Benjamin Herrenschmidt writes:
>
>> On Wed, 2008-09-24 at 08:39 +0200, Arnd Bergmann wrote:
>> > 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 11 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.
>>
>>  .../...
>>
>> It's me or the patch is whitespaces damaged ?
>
>Please don't apply Arnd's patch.  As I said, all we need is this
>one-line change in arch/powerpc/include/asm/systbl.h:
>
>-SYSX_SPU(sys_select,ppc32_select,ppc_select)
>+SYSX_SPU(sys_select,ppc32_select,sys_select)


I have tested the testcase that I have provided at the time of reporting
this issue. But it didn't work as expected with the above patch you
provided.
(Failed to return EINVAL on negative value of n to select())

Please let me know about this. I have tested on PPC32 2.6.23.

-Halesh

>
>Paul.



-------------------------------------------------------------------
This email is confidential and intended only for the use of the =
individual or entity named above and may contain information that is =
privileged. If you are not the intended recipient, you are notified that =
any dissemination, distribution or copying of this email is strictly =
prohibited. If you have received this email in error, please notify us =
immediately by return email or telephone and destroy the original =
message. - This mail is sent via Sony Asia Pacific Mail Gateway.
-------------------------------------------------------------------

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

* RE: [PATCH] remove bogus ppc_select syscall
@ 2008-09-24 13:49 Sadashiiv, Halesh
  0 siblings, 0 replies; 24+ messages in thread
From: Sadashiiv, Halesh @ 2008-09-24 13:49 UTC (permalink / raw)
  To: Sadashiiv, Halesh, Arnd Bergmann, linuxppc-embedded, paulus; +Cc: linuxppc-dev


>One more thing noticed while building the kernel, it gave warning
>
>kernel/power/Kconfig:97:warning: 'select' used by config symbol
>'PM_SLEEP_SMP' refers to undefined symbol 'HOTPLUG_CPU'


Apologies, this is not related to the Fix.
Please ignore my previous post.

>
>Thanks
>Halesh
>
>
>
>
>>-----Original Message-----
>>From: Sadashiiv, Halesh
>>Sent: Wednesday, September 24, 2008 2:42 PM
>>To: 'Arnd Bergmann'; linuxppc-embedded@ozlabs.org; paulus@samba.org
>>Cc: benh@kernel.crashing.org; linuxppc-dev@ozlabs.org
>>Subject: RE: [PATCH] remove bogus ppc_select syscall
>>
>>
>>I have tested the provided patch on PPC32 with little modifications.
>>
>>Test passed on the PPC32.
>>
>>Modification to Arnd's Patch:
>>	In systbl.h you have changed
>>		-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
>>		+SYSCALL(sys_ni_syscall)
>>
>>	That has to be
>>		-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
>>		+SYSCALL(ni_syscall)
>>
>>Which failed the kernel build. It's a typo from your side I think.
>>After fixing it build went fine.
>>
>>Signed-off-by: Halesh Sadashiv <halesh.sadashiv@ap.sony.com>
>>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>>diff --git a/arch/powerpc/include/asm/unistd.h
>>b/arch/powerpc/include/asm/unistd.h
>>index e07d0c7..46107cc 100644
>>--- a/arch/powerpc/include/asm/unistd.h
>>+++ b/arch/powerpc/include/asm/unistd.h
>>@@ -92,7 +92,7 @@
>> #define __NR_settimeofday       79
>> #define __NR_getgroups          80
>> #define __NR_setgroups          81
>>-#define __NR_select             82
>>+/* Number 82 was the old (pre-1.3.x) select */
>> #define __NR_symlink            83
>> #define __NR_oldlstat           84
>> #define __NR_readlink           85
>>diff --git a/arch/powerpc/include/asm/systbl.h
>>b/arch/powerpc/include/asm/systbl.h
>>index f6cc7a4..5a69b32 100644
>>--- a/arch/powerpc/include/asm/systbl.h
>>+++ b/arch/powerpc/include/asm/systbl.h
>>@@ -85,7 +85,7 @@ COMPAT_SYS_SPU(gettimeofday)
>> COMPAT_SYS_SPU(settimeofday)
>> COMPAT_SYS_SPU(getgroups)
>> COMPAT_SYS_SPU(setgroups)
>>-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
>>+SYSCALL(ni_syscall)
>> SYSCALL_SPU(symlink)
>> OLDSYS(lstat)
>> COMPAT_SYS_SPU(readlink)
>>@@ -145,7 +145,7 @@ SYSCALL_SPU(setfsuid)
>> SYSCALL_SPU(setfsgid)
>> SYSCALL_SPU(llseek)
>> COMPAT_SYS_SPU(getdents)
>>-SYSX_SPU(sys_select,ppc32_select,ppc_select)
>>+SYSX_SPU(sys_select,ppc32_select,sys_select)
>> SYSCALL_SPU(flock)
>> SYSCALL_SPU(msync)
>> COMPAT_SYS_SPU(readv)
>>diff --git a/arch/powerpc/kernel/syscalls.c
>>b/arch/powerpc/kernel/syscalls.c index c04832c..c2e6a74 100644
>>--- a/arch/powerpc/kernel/syscalls.c
>>+++ b/arch/powerpc/kernel/syscalls.c
>>@@ -183,31 +183,6 @@ unsigned long sys_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 timeval __user *tvp) -{
>>-	if ( (unsigned long)n >=3D 4096 )
>>-	{
>>-		unsigned long __user *buffer =3D (unsigned long __user
*)n;
>>-		if (!access_ok(VERIFY_READ, 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 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)  {
>>
>>Thanks
>>Halesh
>>
>>
>>
>>
>>>-----Original Message-----
>>>From: Arnd Bergmann [mailto:arnd@arndb.de]
>>>Sent: Wednesday, September 24, 2008 12:09 PM
>>>To: linuxppc-embedded@ozlabs.org; paulus@samba.org
>>>Cc: benh@kernel.crashing.org; Sadashiiv, Halesh;
linuxppc-dev@ozlabs.org
>>>Subject: [PATCH] remove bogus ppc_select syscall
>>>
>>>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 11 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
>>><halesh.sadashiv@ap.sony.com>, 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 <halesh.sadashiv@ap.sony.com>
>>>---
>>>
>>>Halesh, please test this patch to make sure it fixes the problem
>>>you reported. I do not have a ppc32 machine I can try this on.
>>>
>>>diff --git a/arch/powerpc/include/asm/unistd.h
>>b/arch/powerpc/include/asm/unistd.h
>>>index e07d0c7..46107cc 100644
>>>--- a/arch/powerpc/include/asm/unistd.h
>>>+++ b/arch/powerpc/include/asm/unistd.h
>>>@@ -92,7 +92,7 @@
>>> #define __NR_settimeofday       79
>>> #define __NR_getgroups          80
>>> #define __NR_setgroups          81
>>>-#define __NR_select             82
>>>+/* Number 82 was the old (pre-1.3.x) select */
>>> #define __NR_symlink            83
>>> #define __NR_oldlstat           84
>>> #define __NR_readlink           85
>>>diff --git a/arch/powerpc/include/asm/systbl.h
>>>b/arch/powerpc/include/asm/systbl.h
>>>index f6cc7a4..5a69b32 100644
>>>--- a/arch/powerpc/include/asm/systbl.h
>>>+++ b/arch/powerpc/include/asm/systbl.h
>>>@@ -85,7 +85,7 @@ COMPAT_SYS_SPU(gettimeofday)
>>> COMPAT_SYS_SPU(settimeofday)
>>> COMPAT_SYS_SPU(getgroups)
>>> COMPAT_SYS_SPU(setgroups)
>>>-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
>>>+SYSCALL(sys_ni_syscall)
>>> SYSCALL_SPU(symlink)
>>> OLDSYS(lstat)
>>> COMPAT_SYS_SPU(readlink)
>>>@@ -145,7 +145,7 @@ SYSCALL_SPU(setfsuid)
>>> SYSCALL_SPU(setfsgid)
>>> SYSCALL_SPU(llseek)
>>> COMPAT_SYS_SPU(getdents)
>>>-SYSX_SPU(sys_select,ppc32_select,ppc_select)
>>>+SYSX_SPU(sys_select,ppc32_select,sys_select)
>>> SYSCALL_SPU(flock)
>>> SYSCALL_SPU(msync)
>>> COMPAT_SYS_SPU(readv)
>>>diff --git a/arch/powerpc/kernel/syscalls.c
>>>b/arch/powerpc/kernel/syscalls.c
>>>index c04832c..c2e6a74 100644
>>>--- a/arch/powerpc/kernel/syscalls.c
>>>+++ b/arch/powerpc/kernel/syscalls.c
>>>@@ -183,31 +183,6 @@ unsigned long sys_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 timeval __user *tvp)
>>>-{
>>>-	if ( (unsigned long)n >=3D 4096 )
>>>-	{
>>>-		unsigned long __user *buffer =3D (unsigned long __user
*)n;
>>>-		if (!access_ok(VERIFY_READ, 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 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)
>>> {



-------------------------------------------------------------------
This email is confidential and intended only for the use of the =
individual or entity named above and may contain information that is =
privileged. If you are not the intended recipient, you are notified that =
any dissemination, distribution or copying of this email is strictly =
prohibited. If you have received this email in error, please notify us =
immediately by return email or telephone and destroy the original =
message. - This mail is sent via Sony Asia Pacific Mail Gateway.
-------------------------------------------------------------------

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

* RE: [PATCH] remove bogus ppc_select syscall
@ 2008-09-24 13:46 Sadashiiv, Halesh
  0 siblings, 0 replies; 24+ messages in thread
From: Sadashiiv, Halesh @ 2008-09-24 13:46 UTC (permalink / raw)
  To: Sadashiiv, Halesh, Arnd Bergmann, linuxppc-embedded, paulus; +Cc: linuxppc-dev


One more thing noticed while building the kernel, it gave warning

kernel/power/Kconfig:97:warning: 'select' used by config symbol
'PM_SLEEP_SMP' refers to undefined symbol 'HOTPLUG_CPU'

Thanks
Halesh




>-----Original Message-----
>From: Sadashiiv, Halesh
>Sent: Wednesday, September 24, 2008 2:42 PM
>To: 'Arnd Bergmann'; linuxppc-embedded@ozlabs.org; paulus@samba.org
>Cc: benh@kernel.crashing.org; linuxppc-dev@ozlabs.org
>Subject: RE: [PATCH] remove bogus ppc_select syscall
>
>
>I have tested the provided patch on PPC32 with little modifications.
>
>Test passed on the PPC32.
>
>Modification to Arnd's Patch:
>	In systbl.h you have changed
>		-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
>		+SYSCALL(sys_ni_syscall)
>
>	That has to be
>		-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
>		+SYSCALL(ni_syscall)
>
>Which failed the kernel build. It's a typo from your side I think.
>After fixing it build went fine.
>
>Signed-off-by: Halesh Sadashiv <halesh.sadashiv@ap.sony.com>
>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
>diff --git a/arch/powerpc/include/asm/unistd.h
>b/arch/powerpc/include/asm/unistd.h
>index e07d0c7..46107cc 100644
>--- a/arch/powerpc/include/asm/unistd.h
>+++ b/arch/powerpc/include/asm/unistd.h
>@@ -92,7 +92,7 @@
> #define __NR_settimeofday       79
> #define __NR_getgroups          80
> #define __NR_setgroups          81
>-#define __NR_select             82
>+/* Number 82 was the old (pre-1.3.x) select */
> #define __NR_symlink            83
> #define __NR_oldlstat           84
> #define __NR_readlink           85
>diff --git a/arch/powerpc/include/asm/systbl.h
>b/arch/powerpc/include/asm/systbl.h
>index f6cc7a4..5a69b32 100644
>--- a/arch/powerpc/include/asm/systbl.h
>+++ b/arch/powerpc/include/asm/systbl.h
>@@ -85,7 +85,7 @@ COMPAT_SYS_SPU(gettimeofday)
> COMPAT_SYS_SPU(settimeofday)
> COMPAT_SYS_SPU(getgroups)
> COMPAT_SYS_SPU(setgroups)
>-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
>+SYSCALL(ni_syscall)
> SYSCALL_SPU(symlink)
> OLDSYS(lstat)
> COMPAT_SYS_SPU(readlink)
>@@ -145,7 +145,7 @@ SYSCALL_SPU(setfsuid)
> SYSCALL_SPU(setfsgid)
> SYSCALL_SPU(llseek)
> COMPAT_SYS_SPU(getdents)
>-SYSX_SPU(sys_select,ppc32_select,ppc_select)
>+SYSX_SPU(sys_select,ppc32_select,sys_select)
> SYSCALL_SPU(flock)
> SYSCALL_SPU(msync)
> COMPAT_SYS_SPU(readv)
>diff --git a/arch/powerpc/kernel/syscalls.c
>b/arch/powerpc/kernel/syscalls.c index c04832c..c2e6a74 100644
>--- a/arch/powerpc/kernel/syscalls.c
>+++ b/arch/powerpc/kernel/syscalls.c
>@@ -183,31 +183,6 @@ unsigned long sys_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 timeval __user *tvp) -{
>-	if ( (unsigned long)n >=3D 4096 )
>-	{
>-		unsigned long __user *buffer =3D (unsigned long __user
*)n;
>-		if (!access_ok(VERIFY_READ, 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 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)  {
>
>Thanks
>Halesh
>
>
>
>
>>-----Original Message-----
>>From: Arnd Bergmann [mailto:arnd@arndb.de]
>>Sent: Wednesday, September 24, 2008 12:09 PM
>>To: linuxppc-embedded@ozlabs.org; paulus@samba.org
>>Cc: benh@kernel.crashing.org; Sadashiiv, Halesh;
linuxppc-dev@ozlabs.org
>>Subject: [PATCH] remove bogus ppc_select syscall
>>
>>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 11 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
>><halesh.sadashiv@ap.sony.com>, 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 <halesh.sadashiv@ap.sony.com>
>>---
>>
>>Halesh, please test this patch to make sure it fixes the problem
>>you reported. I do not have a ppc32 machine I can try this on.
>>
>>diff --git a/arch/powerpc/include/asm/unistd.h
>b/arch/powerpc/include/asm/unistd.h
>>index e07d0c7..46107cc 100644
>>--- a/arch/powerpc/include/asm/unistd.h
>>+++ b/arch/powerpc/include/asm/unistd.h
>>@@ -92,7 +92,7 @@
>> #define __NR_settimeofday       79
>> #define __NR_getgroups          80
>> #define __NR_setgroups          81
>>-#define __NR_select             82
>>+/* Number 82 was the old (pre-1.3.x) select */
>> #define __NR_symlink            83
>> #define __NR_oldlstat           84
>> #define __NR_readlink           85
>>diff --git a/arch/powerpc/include/asm/systbl.h
>>b/arch/powerpc/include/asm/systbl.h
>>index f6cc7a4..5a69b32 100644
>>--- a/arch/powerpc/include/asm/systbl.h
>>+++ b/arch/powerpc/include/asm/systbl.h
>>@@ -85,7 +85,7 @@ COMPAT_SYS_SPU(gettimeofday)
>> COMPAT_SYS_SPU(settimeofday)
>> COMPAT_SYS_SPU(getgroups)
>> COMPAT_SYS_SPU(setgroups)
>>-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
>>+SYSCALL(sys_ni_syscall)
>> SYSCALL_SPU(symlink)
>> OLDSYS(lstat)
>> COMPAT_SYS_SPU(readlink)
>>@@ -145,7 +145,7 @@ SYSCALL_SPU(setfsuid)
>> SYSCALL_SPU(setfsgid)
>> SYSCALL_SPU(llseek)
>> COMPAT_SYS_SPU(getdents)
>>-SYSX_SPU(sys_select,ppc32_select,ppc_select)
>>+SYSX_SPU(sys_select,ppc32_select,sys_select)
>> SYSCALL_SPU(flock)
>> SYSCALL_SPU(msync)
>> COMPAT_SYS_SPU(readv)
>>diff --git a/arch/powerpc/kernel/syscalls.c
>>b/arch/powerpc/kernel/syscalls.c
>>index c04832c..c2e6a74 100644
>>--- a/arch/powerpc/kernel/syscalls.c
>>+++ b/arch/powerpc/kernel/syscalls.c
>>@@ -183,31 +183,6 @@ unsigned long sys_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 timeval __user *tvp)
>>-{
>>-	if ( (unsigned long)n >=3D 4096 )
>>-	{
>>-		unsigned long __user *buffer =3D (unsigned long __user
*)n;
>>-		if (!access_ok(VERIFY_READ, 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 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)
>> {



-------------------------------------------------------------------
This email is confidential and intended only for the use of the =
individual or entity named above and may contain information that is =
privileged. If you are not the intended recipient, you are notified that =
any dissemination, distribution or copying of this email is strictly =
prohibited. If you have received this email in error, please notify us =
immediately by return email or telephone and destroy the original =
message. - This mail is sent via Sony Asia Pacific Mail Gateway.
-------------------------------------------------------------------

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

* RE: [PATCH] remove bogus ppc_select syscall
@ 2008-09-24  9:12 Sadashiiv, Halesh
  0 siblings, 0 replies; 24+ messages in thread
From: Sadashiiv, Halesh @ 2008-09-24  9:12 UTC (permalink / raw)
  To: Arnd Bergmann, linuxppc-embedded, paulus; +Cc: linuxppc-dev


I have tested the provided patch on PPC32 with little modifications.

Test passed on the PPC32.

Modification to Arnd's Patch:=20
	In systbl.h you have changed=20
		-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
		+SYSCALL(sys_ni_syscall)

	That has to be
		-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
		+SYSCALL(ni_syscall)

Which failed the kernel build. It's a typo from your side I think.
After fixing it build went fine.

Signed-off-by: Halesh Sadashiv <halesh.sadashiv@ap.sony.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/powerpc/include/asm/unistd.h
b/arch/powerpc/include/asm/unistd.h
index e07d0c7..46107cc 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -92,7 +92,7 @@
 #define __NR_settimeofday       79
 #define __NR_getgroups          80
 #define __NR_setgroups          81
-#define __NR_select             82
+/* Number 82 was the old (pre-1.3.x) select */
 #define __NR_symlink            83
 #define __NR_oldlstat           84
 #define __NR_readlink           85
diff --git a/arch/powerpc/include/asm/systbl.h
b/arch/powerpc/include/asm/systbl.h
index f6cc7a4..5a69b32 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -85,7 +85,7 @@ COMPAT_SYS_SPU(gettimeofday)
 COMPAT_SYS_SPU(settimeofday)
 COMPAT_SYS_SPU(getgroups)
 COMPAT_SYS_SPU(setgroups)
-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
+SYSCALL(ni_syscall)
 SYSCALL_SPU(symlink)
 OLDSYS(lstat)
 COMPAT_SYS_SPU(readlink)
@@ -145,7 +145,7 @@ SYSCALL_SPU(setfsuid)
 SYSCALL_SPU(setfsgid)
 SYSCALL_SPU(llseek)
 COMPAT_SYS_SPU(getdents)
-SYSX_SPU(sys_select,ppc32_select,ppc_select)
+SYSX_SPU(sys_select,ppc32_select,sys_select)
 SYSCALL_SPU(flock)
 SYSCALL_SPU(msync)
 COMPAT_SYS_SPU(readv)
diff --git a/arch/powerpc/kernel/syscalls.c
b/arch/powerpc/kernel/syscalls.c index c04832c..c2e6a74 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -183,31 +183,6 @@ unsigned long sys_mmap(unsigned long addr, size_t
len,
 	return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
}
=20
-#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 timeval __user *tvp) -{
-	if ( (unsigned long)n >=3D 4096 )
-	{
-		unsigned long __user *buffer =3D (unsigned long __user
*)n;
-		if (!access_ok(VERIFY_READ, 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 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)  {

Thanks
Halesh




>-----Original Message-----
>From: Arnd Bergmann [mailto:arnd@arndb.de]
>Sent: Wednesday, September 24, 2008 12:09 PM
>To: linuxppc-embedded@ozlabs.org; paulus@samba.org
>Cc: benh@kernel.crashing.org; Sadashiiv, Halesh;
linuxppc-dev@ozlabs.org
>Subject: [PATCH] remove bogus ppc_select syscall
>
>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 11 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
><halesh.sadashiv@ap.sony.com>, 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 <halesh.sadashiv@ap.sony.com>
>---
>
>Halesh, please test this patch to make sure it fixes the problem
>you reported. I do not have a ppc32 machine I can try this on.
>
>diff --git a/arch/powerpc/include/asm/unistd.h
b/arch/powerpc/include/asm/unistd.h
>index e07d0c7..46107cc 100644
>--- a/arch/powerpc/include/asm/unistd.h
>+++ b/arch/powerpc/include/asm/unistd.h
>@@ -92,7 +92,7 @@
> #define __NR_settimeofday       79
> #define __NR_getgroups          80
> #define __NR_setgroups          81
>-#define __NR_select             82
>+/* Number 82 was the old (pre-1.3.x) select */
> #define __NR_symlink            83
> #define __NR_oldlstat           84
> #define __NR_readlink           85
>diff --git a/arch/powerpc/include/asm/systbl.h
>b/arch/powerpc/include/asm/systbl.h
>index f6cc7a4..5a69b32 100644
>--- a/arch/powerpc/include/asm/systbl.h
>+++ b/arch/powerpc/include/asm/systbl.h
>@@ -85,7 +85,7 @@ COMPAT_SYS_SPU(gettimeofday)
> COMPAT_SYS_SPU(settimeofday)
> COMPAT_SYS_SPU(getgroups)
> COMPAT_SYS_SPU(setgroups)
>-SYSX(sys_ni_syscall,sys_ni_syscall,ppc_select)
>+SYSCALL(sys_ni_syscall)
> SYSCALL_SPU(symlink)
> OLDSYS(lstat)
> COMPAT_SYS_SPU(readlink)
>@@ -145,7 +145,7 @@ SYSCALL_SPU(setfsuid)
> SYSCALL_SPU(setfsgid)
> SYSCALL_SPU(llseek)
> COMPAT_SYS_SPU(getdents)
>-SYSX_SPU(sys_select,ppc32_select,ppc_select)
>+SYSX_SPU(sys_select,ppc32_select,sys_select)
> SYSCALL_SPU(flock)
> SYSCALL_SPU(msync)
> COMPAT_SYS_SPU(readv)
>diff --git a/arch/powerpc/kernel/syscalls.c
>b/arch/powerpc/kernel/syscalls.c
>index c04832c..c2e6a74 100644
>--- a/arch/powerpc/kernel/syscalls.c
>+++ b/arch/powerpc/kernel/syscalls.c
>@@ -183,31 +183,6 @@ unsigned long sys_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 timeval __user *tvp)
>-{
>-	if ( (unsigned long)n >=3D 4096 )
>-	{
>-		unsigned long __user *buffer =3D (unsigned long __user
*)n;
>-		if (!access_ok(VERIFY_READ, 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 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)
> {



-------------------------------------------------------------------
This email is confidential and intended only for the use of the =
individual or entity named above and may contain information that is =
privileged. If you are not the intended recipient, you are notified that =
any dissemination, distribution or copying of this email is strictly =
prohibited. If you have received this email in error, please notify us =
immediately by return email or telephone and destroy the original =
message. - This mail is sent via Sony Asia Pacific Mail Gateway.
-------------------------------------------------------------------

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

end of thread, other threads:[~2008-10-13 21:40 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-09-23  7:16 Regarding select() on PPC Sadashiiv, Halesh
2008-09-23  9:07 ` Benjamin Herrenschmidt
2008-09-24  6:23   ` Arnd Bergmann
2008-09-24  6:28     ` Benjamin Herrenschmidt
2008-09-24  6:45       ` Arnd Bergmann
2008-09-24  7:08         ` Benjamin Herrenschmidt
2008-09-24 21:46           ` Benjamin Herrenschmidt
2008-09-24  6:39     ` [PATCH] remove bogus ppc_select syscall Arnd Bergmann
2008-09-24 16:29       ` Paul Mackerras
2008-09-24 17:03         ` Arnd Bergmann
2008-10-10  7:40           ` Paul Mackerras
2008-10-10  8:44             ` Arnd Bergmann
2008-10-10  4:29       ` Benjamin Herrenschmidt
2008-10-10  7:43         ` Paul Mackerras
2008-10-10  8:45           ` Arnd Bergmann
2008-10-10 23:17           ` Benjamin Herrenschmidt
2008-10-12  8:58             ` Arnd Bergmann
2008-10-13  2:42             ` i2o driver work with AMCC 460EX? Ayman El-Khashab
2008-09-24  9:12 [PATCH] remove bogus ppc_select syscall Sadashiiv, Halesh
2008-09-24 13:46 Sadashiiv, Halesh
2008-09-24 13:49 Sadashiiv, Halesh
2008-10-13 11:59 Sadashiiv, Halesh
2008-10-13 21:40 ` Paul Mackerras
2008-10-13 12:18 Sadashiiv, Halesh

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).