bug on ppc (and potentially other arches) in setgroups.c

[Mike Frysinger]

On Wednesday 01 July 2009 22:44:58 Phil Estes wrote:
> Before I open a bug, I thought I would look for clarification of what I
> believe I'm seeing in libc/sysdeps/linux/common/setgroups.c.  I think
> it's possible that the code was originally i386-centric and has not been
> updated as other architectures are added.

it isnt i386 at all.  the 16 vs 32 bitness you note later on is correct, and 
not available in all kernels, hence the logic follows what has largely been 
the standard in practice:
 - if the arch is 64bit, the [gs]etgroups functions have always been 32bit
 - if the arch is 32bit:
  - historically, only 16bit versions were available, thus no xxx32 means the 
wrapper is necessary
  - with xxx32 version, 32bit was added, so the wrapper is not needed

> Background:  the LTP has a testcase "setgroups04"--the purpose of which
> is to try and get a -EFAULT out of the setgroups syscall by passing in a
> known invalid memory value.  This should flow into the kernel and get an
> -EFAULT after a failure of "copy_from_user" of the groups list pointer
> passed to the syscall.

if you want to make sure a test case flows into the kernel, LTP needs to use a 
syscall().  last time i reviewed this failure, i chalked the uClibc behavior 
up as acceptable as the standard does not dictate what must happen if the 
array specified is invalid with getgroups.  in my mind, that means a crash is 
acceptable behavior.  considering uClibc aims for small code, adding if() 
checks for stupid programmers when the checks are not required by POSIX is 
wrong -- let the code blow up.

really the best thing to do would be to pose the question to the POSIX mailing 
lists for clarification (you should also ask about getgroups since the two 
should behave the same and setgroups is not in POSIX).

> In the case of uClibc on the ppc architecture, this means that the code
> flows through the setgroups "wrapper" in setgroups.c which tries to copy
> the passed-in group array to an array of allocated kernel_gid_t
> structs.   In tracing back through why this might be, it seems to be
> that this code would be for the compat syscall for 16-bit UIDs
> (implemented in the kernel in uid16.c); and therefore the kernel gid_t
> might not match the userspace gid_t; hence the copying.  It tries to
> only use this wrapper if __NR_setgroups32 isn't defined, which it is not
> for ppc.  But, on ppc, there is no 16-bit version or compat version of
> the setgroups syscall; therefore, there should also be no logic to
> attempt to perform the copying, which is really not useful on ppc (and
> potentially other architectures which don't have this setgroups vs.
> setgroups32 syscall issue).

are you sure that this is the case ?  or are you assuming "no __NR_setgroups32 
means setgroups has always been 32bit" ?  if you can get verification from 
LKML, then we can look at changing uClibc.  this isnt a ppc32 specific issue 
as i see avr32, mips32, parisc32, and xtensa are in a similar boat.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/uclibc/attachments/20090702/37866469/attachment.pgp>