Messages in this thread | | | Date | Wed, 1 Dec 2010 19:27:47 +0100 | From | Oleg Nesterov <> | Subject | Re: (No subject header) |
| |
On 12/01, Milton Miller wrote: > > On Tue, 30 Nov 2010 about 20:01:29 -0000, Oleg Nesterov wrote: > > Teach get_arg_ptr() to handle compat = T case correctly. > > > #include <asm/uaccess.h> > > #include <asm/mmu_context.h> > > @@ -395,6 +396,18 @@ get_arg_ptr(const char __user * const __ > > { > > const char __user *ptr; > > > > +#ifdef CONFIG_COMPAT > > + if (unlikely(compat)) { > > This should not be marked unlikely. Unlikely tells gcc the path > with over 99% confidence and disables branch predictors on some > architectures. If called from a compat processes this will result > in a mispredicted branch every iteration. Just use if (compat) > and let the hardware branch predictors do their job.
This applies to almost every likely/unlikely, and I think that compat processes should fall into "unlikely category". But I don't really mind, I can remove this hint, I added it mostly as documentation.
> > +#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, > > Shouldn't these be compat_ptr(argv)? (makes a difference on s390)
I'll recheck, but I don't think so. Please note that compat_ptr() accepts "compat_uptr_t", not "compat_uptr_t *".
argv should be correct as a pointer to user-space, otherwise the current code is buggy. For example, compat_do_execve() passes argv to compat_count() which does get_user(argv) without any conversion.
IOW, even if this should be fixed, I think this have nothing to do with this patch. But I'll recheck, thanks.
Oleg.
| |