Re: [LTP] [PATCH] syscalls/setgroups: fix implicit SETGROUPS parameter casting

[chrubis@suse.cz]

> One of parameters to setgroups() syscall is (gid_t *) pointer.
> If TST_USE_COMPAT16_VSYSCALL is defined a pointer to GID_T is passed
> instead (and sizeof(GID_T) < sizeof(gid_t)). It's not safe and
> can result in unaligned access (and SIGBUS) on several platforms.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  testcases/kernel/syscalls/setgroups/compat_16.h   |   22 +++++++++++++++++++-
>  testcases/kernel/syscalls/setgroups/setgroups04.c |    6 ++++-
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/setgroups/compat_16.h b/testcases/kernel/syscalls/setgroups/compat_16.h
> index 0de4e78..35723d6 100644
> --- a/testcases/kernel/syscalls/setgroups/compat_16.h
> +++ b/testcases/kernel/syscalls/setgroups/compat_16.h
> @@ -32,9 +32,27 @@ extern void cleanup(void);
>  #ifdef TST_USE_COMPAT16_SYSCALL
>  
>  long
> -SETGROUPS(size_t gidsetsize, GID_T *list)
> +SETGROUPS(size_t gidsetsize, GID_T *list16)
>  {
> -	return ltp_syscall(__NR_setgroups, gidsetsize, list);
> +	int r;
> +	int i;
> +
> +	gid_t *list32;
> +
> +	list32 = calloc(gidsetsize, sizeof(gid_t));
> +	if (list32 == NULL)
> +		tst_brkm(TBROK | TERRNO, NULL,
> +			"calloc failed to allocate %zu bytes at %s:%d",
> +			gidsetsize * sizeof(gid_t),
> +			__FILE__, __LINE__);
> +
> +	for (i = 0; i < gidsetsize; i++)
> +		list32[i] = list16[i];
> +
> +	r = ltp_syscall(__NR_setgroups, gidsetsize, list32);
> +
> +	free(list32);
> +	return r;
>  }

This looks like the __NR_setgroups is not the compact16 one we want.

Look at the getgroups16 in kernel/uid16.c it calls groups16_from_user()
and that does:


        for (i = 0; i < group_info->ngroups; i++) {
                if (get_user(group, grouplist+i))
                        return  -EFAULT;


The grouplist is of old_gid_t __user *grouplist type so it's unsigned short, so
it works with array of unsigned short which is aligned to unsigned shorts due
to definiton of the GID_T in the test sources, there is no way this
would trigger unaligned access.

And actually the GETGROUPS() seems to be coded to pass list32 and converts it
to list16 which looks just wrong to me as it's inside of
TST_USE_COMPAT16_SYSCALL. It just looks to me like we have wrong syscall
number to begin with.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list