linux-kernel - Re: (No subject header)

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:	Wed, 1 Dec 2010 19:27:47 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Milton Miller <miltonm@....com>
Cc:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	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>
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.

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