Re: [PATCH] [BZ #19408] Fix linux personality syscall wrapper

["Dmitry V. Levin" <ldv at altlinux dot org>]

On Tue, Dec 29, 2015 at 06:50:07PM -0500, Mike Frysinger wrote:
> On 30 Dec 2015 02:27, Dmitry V. Levin wrote:
> > On Tue, Dec 29, 2015 at 04:42:18PM -0500, Mike Frysinger wrote:
> > > On 28 Dec 2015 04:09, Dmitry V. Levin wrote:
> > > > --- /dev/null
> > > > +++ b/sysdeps/unix/sysv/linux/personality.c
> > > >
> > > > +  INTERNAL_SYSCALL_DECL (err);
> > > > +  long ret = INTERNAL_SYSCALL (personality, err, 1, persona);
> > > > +
> > > > +  /* Starting with kernel commit v2.6.29-6609-g11d06b2, personality syscall
> > > > +     never fails to set the personality.  However, due to architecture
> > > > +     limitations of 32-bit kernels, personality syscall still may return
> > > > +     an "error".  */
> > > > +  if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (ret, err)))
> > > > +    ret = -INTERNAL_SYSCALL_ERRNO (ret, err);
> > > > +  return ret;
> > > > +}
> > > 
> > > INTERNAL_SYSCALL returns the raw value from the kernel, so why do you
> > > need to test it directly ?
> > 
> > While some architectures (including x86/x86_64) use negated errno semantics,
> > others (e.g. mips, nios2, powerpc, and sparc) use dedicated registers to return
> > an error condition.  In the latter case, the kernel syscall exit code negates
> > the value returned by sys_personality and returns it to userspace as an
> > "errno".  It has to be negated again to recover the value returned by
> > sys_personality.
> 
> i'm aware of some arches doing split error paths, but doesn't the kernel
> already handle this ?

Unfortunately, it doesn't.

> i'm not seeing this sort of logic for ptrace or
> mmap calls where any value is valid.

mmap doesn't have this issue because you cannot trick it to return
an address >= -4095.

ptrace doesn't have this issue because it doesn't return arbitrary values,
PTRACE_PEEK{TEXT,DATA,USER) return value is stored in memory, see
sysdeps/unix/sysv/linux/ptrace.c.

> i guess the commit could use clarification here.

The subject is tricky and the commit is quite lengthy already.
What would you like to be clarified?


-- 
ldv

Attachment: pgpBUckNzjYyu.pgp
Description: PGP signature