Re: [Qemu-devel] [PATCH] linux-user, fix getgroups/getgroups32 when both args are zero.
On [Sat, 23.02.2008 09:25], Kirill A. Shutemov wrote: > On [Sat, 23.02.2008 12:00], Takashi Yoshii wrote: > > getgroups() and getgroups32() returns NGROUPS_MAX when both its two args are > > zero. But because we pass a ptr to allocated space as 2nd arg, this function > > are interfered. The patch attached fixed it. > > /yoshii > > --- > > linux-user/syscall.c: fix getgroups{,32} when both args are zero. > > > > My version of patch to fix same problem: > > getgroups: return total number of supplementary group IDs for the > process if size == 0 > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index ad97871..96a11a9 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -5029,6 +5029,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > > grouplist = alloca(gidsetsize * sizeof(gid_t)); > ret = get_errno(getgroups(gidsetsize, grouplist)); > +if (gidsetsize == 0) > +break; > if (!is_error(ret)) { > target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize > * 2, 0); > if (!target_grouplist) > @@ -5179,6 +5181,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > > grouplist = alloca(gidsetsize * sizeof(gid_t)); > ret = get_errno(getgroups(gidsetsize, grouplist)); > +if (gidsetsize == 0) > +break; > if (!is_error(ret)) { > target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize > * 4, 0); > if (!target_grouplist) { > > Another patch for getgroups: > > Trivial optimization of getgroups syscall implementation: > swap only returned groups, not all group in list. > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 6f2872f..ad97871 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -5033,7 +5033,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize > * 2, 0); > if (!target_grouplist) > goto efault; > -for(i = 0;i < gidsetsize; i++) > +for(i = 0;i < ret; i++) > target_grouplist[i] = tswap16(grouplist[i]); > unlock_user(target_grouplist, arg2, gidsetsize * 2); > } > @@ -5185,7 +5185,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > ret = -TARGET_EFAULT; > goto fail; > } > -for(i = 0;i < gidsetsize; i++) > +for(i = 0;i < ret; i++) > target_grouplist[i] = tswap32(grouplist[i]); > unlock_user(target_grouplist, arg2, gidsetsize * 4); > } > > It makes getgroups much faster in some cases. Can anybody commit it? -- Regards, Kirill A. Shutemov + Belarus, Minsk + Velesys Ltd, http://www.velesys.com/ + ALT Linux Team, http://www.altlinux.com/ signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH] linux-user, fix getgroups/getgroups32 when both args are zero.
Hi, > My version of patch to fix same problem: Oh, yes. You are right. I have read manpage again, and I found it says, if size is zero, list is not modified, but ... So, mine is wrong. We should check only "size". /yoshii
Re: [Qemu-devel] [PATCH] linux-user, fix getgroups/getgroups32 when both args are zero.
On [Sat, 23.02.2008 12:00], Takashi Yoshii wrote: > getgroups() and getgroups32() returns NGROUPS_MAX when both its two args are > zero. But because we pass a ptr to allocated space as 2nd arg, this function > are interfered. The patch attached fixed it. > /yoshii > --- > linux-user/syscall.c: fix getgroups{,32} when both args are zero. > My version of patch to fix same problem: getgroups: return total number of supplementary group IDs for the process if size == 0 diff --git a/linux-user/syscall.c b/linux-user/syscall.c index ad97871..96a11a9 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -5029,6 +5029,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, grouplist = alloca(gidsetsize * sizeof(gid_t)); ret = get_errno(getgroups(gidsetsize, grouplist)); +if (gidsetsize == 0) +break; if (!is_error(ret)) { target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 2, 0); if (!target_grouplist) @@ -5179,6 +5181,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, grouplist = alloca(gidsetsize * sizeof(gid_t)); ret = get_errno(getgroups(gidsetsize, grouplist)); +if (gidsetsize == 0) +break; if (!is_error(ret)) { target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 4, 0); if (!target_grouplist) { Another patch for getgroups: Trivial optimization of getgroups syscall implementation: swap only returned groups, not all group in list. diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 6f2872f..ad97871 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -5033,7 +5033,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 2, 0); if (!target_grouplist) goto efault; -for(i = 0;i < gidsetsize; i++) +for(i = 0;i < ret; i++) target_grouplist[i] = tswap16(grouplist[i]); unlock_user(target_grouplist, arg2, gidsetsize * 2); } @@ -5185,7 +5185,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, ret = -TARGET_EFAULT; goto fail; } -for(i = 0;i < gidsetsize; i++) +for(i = 0;i < ret; i++) target_grouplist[i] = tswap32(grouplist[i]); unlock_user(target_grouplist, arg2, gidsetsize * 4); } It makes getgroups much faster in some cases. -- Regards, Kirill A. Shutemov + Belarus, Minsk + Velesys Ltd, http://www.velesys.com/ + ALT Linux Team, http://www.altlinux.com/ signature.asc Description: Digital signature
[Qemu-devel] [PATCH] linux-user, fix getgroups/getgroups32 when both args are zero.
getgroups() and getgroups32() returns NGROUPS_MAX when both its two args are zero. But because we pass a ptr to allocated space as 2nd arg, this function are interfered. The patch attached fixed it. /yoshii --- linux-user/syscall.c: fix getgroups{,32} when both args are zero. diff --git a/linux-user/syscall.c b/linux-user/syscall.c --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -5185,7 +5185,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, ret = get_errno(setregid(low2highgid(arg1), low2highgid(arg2))); break; case TARGET_NR_getgroups: -{ +if (arg1 == 0 && arg2 == 0) +ret = get_errno(getgroups(0, 0)); +else { int gidsetsize = arg1; uint16_t *target_grouplist; gid_t *grouplist; @@ -5335,7 +5337,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, #endif #ifdef TARGET_NR_getgroups32 case TARGET_NR_getgroups32: -{ +if (arg1 == 0 && arg2 == 0) +ret = get_errno(getgroups(0, 0)); +else { int gidsetsize = arg1; uint32_t *target_grouplist; gid_t *grouplist;