Re: [PATCH] i386: clean up user_mode() use

Re: [PATCH] i386: clean up user_mode() use

[Ingo Molnar]

* pageexec@freemail.hu <pageexec@freemail.hu> wrote:

> on i386 there're two macros used for testing the userland execution 
> mode: user_mode() and user_mode_vm(), which is not intuitive as on 
> many (all?) other architectures there's only user_mode() and 
> architecture independent code is written with user_mode() only, and 
> even on i386 someone can make the wrong assumption that user_mode() 
> works as it does on other archs.
> 
> two cases in point: 
> drivers/oprofile/cpu_buffer.c:oprofile_add_sample() uses user_mode() 
> which can lead to incorrect results if the interrupted task was in v86 
> mode with a code segment fooling the user_mode() selector RPL check. 
> also, arch/i386/kernel/kprobes.c:kprobe_exceptions_notify() used to 
> use user_mode() whereas it really meant user_mode_vm(), this is in 
> fact incorrect until 2.6.17.
> 
> to avoid such mistakes in the future, the suggested solution is to 
> make user_mode() on i386 consistent with the generic expectation and 
> make it detect any user mode execution context, that is, it should 
> take the role of user_mode_vm() and a new user_mode_novm() is 
> introduced for the i386 specific cases where v86 mode can be excluded. 
> in short, the patch simply does a
> 
>   user_mode_vm -> user_mode
>   user_mode    -> user_mode_novm
> 
> substitution as appropriate.
> 
> Signed-off-by: PaX Team <pageexec@freemail.hu>

agreed!

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: [PATCH] i386: clean up user_mode() use

[Ingo Molnar]

* Linus Torvalds <torvalds@osdl.org> wrote:

> On Sun, 2 Jul 2006, pageexec@freemail.hu wrote:
> > 
> > the fact that arch *independent* code makes use of user_mode() was 
> > apparently lost on you.
> 
> Argh. Yes, there's one single use, apparently.
> 
> Sad.

but even for new non-generic per-arch code a couple of security holes 
were introduced due to the existing semantics of user_mode(). So this 
indeed changes semantics and it's totally intentional.

so i think we should bite the bullet and should apply this patch. We 
have a constant influx of user_mode() using code and as practice has 
shown it, people dont really consider the vm86 angle of it.

	Ingo

Re: [PATCH] i386: clean up user_mode() use

[Linus Torvalds]

On Sun, 2 Jul 2006, pageexec@freemail.hu wrote:
> 
> the fact that arch *independent* code makes use of user_mode() was
> apparently lost on you.

Argh. Yes, there's one single use, apparently.

Sad.

		Linus

Re: [PATCH] i386: clean up user_mode() use

[Linus Torvalds]

On Sun, 2 Jul 2006, Chuck Ebbert wrote:
> > > 
> > >   user_mode_vm -> user_mode
> > >   user_mode    -> user_mode_novm

No. Horrible.

I'm disgusted that somebody really would even consider something like 
this. It CHANGES SEMANTICS for the same name, which is a maintenance 
nightmare and a security bug. Now some patch assumes the new behaviour, 
gets back-ported, and is subtly broken.

If you want to get rid of the "user_mode()" thing, don't re-use the name 
with somethign that means something else.

Just do

	user_mode() -> user_mode_novm86()

instead, and leave the old macro entirely behind.

		Linus

Re: [PATCH] i386: clean up user_mode() use

[Chuck Ebbert]

In-Reply-To: <20060702133718.GA27549@elte.hu>

On Sun, 2 Jul 2006 15:37:18 +0200, Ingo Molnar wrote:

> > to avoid such mistakes in the future, the suggested solution is to 
> > make user_mode() on i386 consistent with the generic expectation and 
> > make it detect any user mode execution context, that is, it should 
> > take the role of user_mode_vm() and a new user_mode_novm() is 
> > introduced for the i386 specific cases where v86 mode can be excluded. 
> > in short, the patch simply does a
> > 
> >   user_mode_vm -> user_mode
> >   user_mode    -> user_mode_novm
> > 
> > substitution as appropriate.
> > 
> > Signed-off-by: PaX Team <pageexec@freemail.hu>
> 
> agreed!
> 
> Acked-by: Ingo Molnar <mingo@elte.hu>

Please make that second one

        user_mode_novm86

Otherwise people might think it means "user mode no virtual memory."

-- 
Chuck
 "You can't read a newspaper if you can't read."  --George W. Bush