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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 2 Oct 2014 16:23:13 +0100
From:	Catalin Marinas <catalin.marinas@....com>
To:	Andrew Pinski <apinski@...ium.com>
Cc:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"pinskia@...il.com" <pinskia@...il.com>
Subject: Re: [PATCH 21/24] ARM64:ILP32: Use a seperate syscall table as a few
 syscalls need to be using the compat syscalls

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/