On 02/25, Linus Torvalds wrote:
>
> On Fri, Feb 25, 2011 at 9:53 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> > Teach get_arg_ptr() to handle compat = T case correctly.
>
> Does it?
I think it does.
> > +#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;
Personally I don't really like this union, to me "void __user*" looks
better, but I won't insist.
> int compat_do_execve(char *filename,
> compat_uptr_t argv,
> compat_uptr_t envp,
> {
> return do_execve_common(filename,
> compat_ptr(argv), compat_ptr(envp), regs);
Indeed! But, again, this has nothing to do with this series. We can
do this later and change the callers in arch/.
> 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.
Once again, to me "void __user*" looks better (just simpler). In this
case get_arg_ptr() becomes (without const/__user for the clarity)
void *get_arg_ptr(void **argv, int argc, bool compat)
{
char *ptr;
#ifdef CONFIG_COMPAT
if (unlikely(compat)) {
compat_uptr_t *a = argv;
compat_uptr_t p;
if (get_user(p, a + argc))
return ERR_PTR(-EFAULT);
return compat_ptr(p);
}
#endif
if (get_user(ptr, argv + argc))
return ERR_PTR(-EFAULT);
return ptr;
}
Otherwise, get_arg_ptr() should return conditional_user_ptr_t as well,
this looks like the unnecessary complication to me, but of course this
is subjective.
So, what do you think?
Oleg.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>