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.
--
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 policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>