Re: [PATCH consolidate sys_ptrace

[Blaisorblade <blaisorblade@yahoo.it>]

Andrew Morton <akpm <at> osdl.org> writes:

> 
> David Howells <dhowells <at> redhat.com> wrote:
> >
> > Christoph Hellwig <hch <at> lst.de> wrote:
> > 

> >  (1) Make a sys_ptrace() *jump* to arch_ptrace() instead of calling it, thus
> >      obviating the extra return step.
> > 

> If we can remove the lock_kernel() and move the final put_task_struct()
> into each arch_ptrace() then we can end sys_ptrace() with

> 	return arch_ptrace(....);

> and with luck, gcc will convert it into a tailcall for us.

Yep, it can do it, especially if CONFIG_REGPARM is enabled.

> It's probably not the first place to start doing such optimisation tho.

Boys, you risk being burned. I'm sorry I'll have to teach you a lesson. I'm 
especially sorry because I had to learn it the hard way...

prevent_tail_call is there for a reason (grep for it in kernel/exit.c)

* If you do:

int do_foo(params...) {
...
}

asmlinkage int sys_foo(params...) {
        return do_foo(a_new_param, params...);
}

* and do_foo and sys_foo have different prototypes (such as in the example or in 
the patch),

THEN

GCC can reorder/change parameters of sys_foo on the stack, to make them match 
the do_foo call.

Since those parameters are afterwards restored into userspace registers (which 
are supposed to be unchanged), we get userspace breakage.

But only if userspace uses the registers afterwards, and if it calls with int 
0x80 (there's no restoring otherwise, or something such).

I know this because I did once this exact error, and it was very hard to 
diagnose (actually, it was in a UML-patch and I got breakage in UML). Also, it 
was triggered only when CONFIG_REGPARM is enabled. If needed, I can point out 
real examples (but you already should know).