CONFIG_REGPARM - prevent_tail_call doubts (context: SKAS3 bug in detail)

From: Blaisorblade <blaisorblade@yahoo.it>
To: user-mode-linux-devel@lists.sourceforge.net
Cc: Jeff Dike <jdike@addtoit.com>,
	Bodo Stroesser <bstroesser@fujitsu-siemens.com>,
	Alexander Viro <viro@parcelfarce.linux.theplanet.co.uk>,
	LKML <linux-kernel@vger.kernel.org>
Subject: CONFIG_REGPARM - prevent_tail_call doubts (context: SKAS3 bug in detail)
Date: Tue, 12 Jul 2005 00:18:50 +0200	[thread overview]
Message-ID: <200507120018.51436.blaisorblade@yahoo.it> (raw)

I just diagnosed (and announced) a big bug affecting the SKAS3 patch: namely, 
syscall parameter values stored in registers may be corrupted for some 
syscalls on return, when called through int 0x80, and when CONFIG_REGPARM is 
enabled.

Ok, the diagnosys of the SKAS3 bug I just noticed is that simply, this 
construct:

int do_foo(params...) {
}

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

does not work, because sys_foo() is optimized to reorder parameters on the 
stack and to tail-call do_foo. The corrupted parameters on the stack will 
then be restored (when calling with int $0x80) inside the userspace 
registers. From entry.S, especially from this comment:

	/* if something modifies registers it must also disable sysexit */

it's clear that when using SYSENTER registers are not restored (even verified 
through sys_iopl() code, which touched EFLAGS).

I've used prevent_tail_call to fix this, and it works (verified with tests and 
assembly inspection). I even think I've understood why it works... it's clear 
why it disallows tail call, but I thought that GCC could create a normal call 
reusing some space from the stack frame of sys_foo, to create the stack frame 
of do_foo... it's just that it wouldn't improve speed.

This construct is used for four syscalls (sys_mmap2, old_mmap, sys_mprotect, 
sys_modify_ldt) and I verified the bug for all sys_mmap2 and sys_mprotect, 
and I'm sure about modify_ldt because the compiled code is identical to 
sys_mprotect().

I initially noticed this with the errno-vs-NPTL fix I and Al Viro discussed 
some time ago: it used indeed mmap2() and triggered the bug.

Luckily, strace reads the correct data (since syscall params are read before 
the syscall is done) so I couldn't do anything else than understand something 
bad was happening.
-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade

___________________________________ 
Yahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it