Re: [PATCH 21/24] ARM64:ILP32: Use a seperate syscall table as a few syscalls need to be using the compat syscalls

[Catalin Marinas]

On Wed, Sep 03, 2014 at 10:19:15PM +0100, Andrew Pinski wrote:
> +/*
> + * sigaltstack needs some special handling as the
> + * padding for stack_t might not be non-zero.
> + */
> +long ilp32_sys_sigaltstack(const stack_t __user *uss_ptr,
> +                          stack_t __user *uoss_ptr)
> +{
> +       stack_t uss, uoss;
> +       int ret;
> +       mm_segment_t seg;
> +
> +       if (uss_ptr) {
> +               if (!access_ok(VERIFY_READ, uss_ptr, sizeof(*uss_ptr)))
> +                       return -EFAULT;
> +               if (__get_user(uss.ss_sp, &uss_ptr->ss_sp) |
> +                       __get_user(uss.ss_flags, &uss_ptr->ss_flags) |
> +                       __get_user(uss.ss_size, &uss_ptr->ss_size))
> +                       return -EFAULT;
> +               /* Zero extend the sp address and the size. */
> +               uss.ss_sp = (void *)(uintptr_t)(unsigned int)(uintptr_t)uss.ss_sp;

Do you need the first (uintptr_t) cast here?

> +               uss.ss_size = (size_t)(unsigned int)uss.ss_size;
> +       }
> +       seg = get_fs();
> +       set_fs(KERNEL_DS);
> +       /*
> +        * Note we need to use uoss as we have changed the segment to the
> +        * kernel one so passing an user one around is wrong.
> +        */

I wonder whether it would be safe to just zero the top 32 bits of ss_sp
on the user stack directly. Would we ever expect this to be read-only?

> +       ret = sys_sigaltstack((stack_t __force __user *) (uss_ptr ? &uss : NULL),
> +                             (stack_t __force __user *) &uoss);

Nitpick: there shouldn't be any space after the type cast.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/