linux-kernel - Re: [PATCH 3/5] exec: unify compat_do

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]

Message-ID: <AANLkTik8epq5cx8n=k6ocMUfbg9kkUAZ8KL7ZiG4UuoU@mail.gmail.com>
Date:	Fri, 25 Feb 2011 11:10:46 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	LKML <linux-kernel@...r.kernel.org>,
	linux-mm <linux-mm@...ck.org>, pageexec@...email.hu,
	Solar Designer <solar@...nwall.com>,
	Eugene Teo <eteo@...hat.com>,
	Brad Spengler <spender@...ecurity.net>,
	Roland McGrath <roland@...hat.com>,
	Milton Miller <miltonm@....com>
Subject: Re: [PATCH 3/5] exec: unify compat_do_execve() code

On Fri, Feb 25, 2011 at 9:53 AM, Oleg Nesterov <oleg@...hat.com> wrote:
> Teach get_arg_ptr() to handle compat = T case correctly.

Does it?

> +#ifdef CONFIG_COMPAT
> +int compat_do_execve(char *filename,
> +       compat_uptr_t __user *argv,
> +       compat_uptr_t __user *envp,
> +       struct pt_regs *regs)
> +{
> +       return do_execve_common(filename,
> +                               (void __user *)argv, (void __user*)envp,
> +                               regs, true);
> +}
> +#endif

I really suspect this should be something like

  typedef union {
     compat_uptr_t compat;
     const char __user *native;
   } conditional_user_ptr_t;

  ...

  int compat_do_execve(char *filename,
                  compat_uptr_t argv,
                  compat_uptr_t envp,
                  struct pt_regs *regs)
   {
             return do_execve_common(filename,
                      compat_ptr(argv), compat_ptr(envp), regs);

where that 'do_execve_common()' takes it's arguments as

    union conditional_user_ptr_t __user *argv,
    union conditional_user_ptr_t __user *envp

and then in get_arg_ptr() we do the proper union member dereference
depending on the "compat" flag.

THAT would actually have the type system help us track what is
actually going on, and would clarify the rules. It would also make it
clear that "do_execve_common()" does *not* take some kind of random
pointer to user space (much less a "const char __user *const char
__user *"). It really does take a pointer to user space, but what that
pointer contains in turn depends on the "compat" flag.

IOW, it really acts as a pointer to a user-space union, and I think
we'd be better off having the type show that.

                  Linus
--
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/