The x86 TLS cleanup in commit efd1ca52d04d2f6df337a3332cee56cd60e6d4c4
made the sys_set_thread_area and sys_get_thread_area functions ripe for
tail call optimization. If the compiler chooses to use it for them, it
can clobber the user trap frame because these are asmlinkage functions.
Signed-off-by: Roland McGrath <[email protected]>
---
arch/x86/kernel/tls.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/tls.c b/arch/x86/kernel/tls.c
index 6dfd4e7..022bcaa 100644
--- a/arch/x86/kernel/tls.c
+++ b/arch/x86/kernel/tls.c
@@ -91,7 +91,9 @@ int do_set_thread_area(struct task_struct *p, int idx,
asmlinkage int sys_set_thread_area(struct user_desc __user *u_info)
{
- return do_set_thread_area(current, -1, u_info, 1);
+ int ret = do_set_thread_area(current, -1, u_info, 1);
+ prevent_tail_call(ret);
+ return ret;
}
@@ -139,7 +141,9 @@ int do_get_thread_area(struct task_struct *p, int idx,
asmlinkage int sys_get_thread_area(struct user_desc __user *u_info)
{
- return do_get_thread_area(current, -1, u_info);
+ int ret = do_get_thread_area(current, -1, u_info);
+ prevent_tail_call(ret);
+ return ret;
}
int regset_tls_active(struct task_struct *target,
* Roland McGrath <[email protected]> wrote:
> The x86 TLS cleanup in commit efd1ca52d04d2f6df337a3332cee56cd60e6d4c4
> made the sys_set_thread_area and sys_get_thread_area functions ripe
> for tail call optimization. If the compiler chooses to use it for
> them, it can clobber the user trap frame because these are asmlinkage
> functions.
thanks, applied.
> asmlinkage int sys_set_thread_area(struct user_desc __user *u_info)
> {
> - return do_set_thread_area(current, -1, u_info, 1);
> + int ret = do_set_thread_area(current, -1, u_info, 1);
> + prevent_tail_call(ret);
> + return ret;
i'm wondering, have you seen this happen in practice? We use
sys_set_thread_area() for every new task started up. I guess we havent
seen problems in the field yet because this early during startup tasks
do not normally receive signals? (or if they do they are fatal and no
user signal context is used.)
btw., gcc 4.2.3 doesnt do it due to CONFIG_FRAME_POINTERS=y here, and
most distros turn on frame pointers.
btw., this whole thing of us having to notice such tail-optimization
incidents is totally fragile and unreliable. Shouldnt there be a "dont
tail-optimize me" attribute which we could stick into asmlinkage?
Perhaps sparse could detect asmlinkage functions that do not do
prevent_tail_call()s?
Ingo
> i'm wondering, have you seen this happen in practice? We use
> sys_set_thread_area() for every new task started up. I guess we havent
> seen problems in the field yet because this early during startup tasks
> do not normally receive signals? (or if they do they are fatal and no
> user signal context is used.)
Tomasz saw it. I don't know what compiler or exact options to it he used.
> btw., this whole thing of us having to notice such tail-optimization
> incidents is totally fragile and unreliable. Shouldnt there be a "dont
> tail-optimize me" attribute which we could stick into asmlinkage?
I agree. It's come up before. I'll talk to compiler folks about it again.
> Perhaps sparse could detect asmlinkage functions that do not do
> prevent_tail_call()s?
That sounds like a good idea to me.
Thanks,
Roland
Dnia Wednesday 27 of February 2008, Roland McGrath napisa?:
> > i'm wondering, have you seen this happen in practice? We use
> > sys_set_thread_area() for every new task started up. I guess we havent
> > seen problems in the field yet because this early during startup tasks
> > do not normally receive signals? (or if they do they are fatal and no
> > user signal context is used.)
>
> Tomasz saw it. I don't know what compiler or exact options to it he used.
>
VMware image from http://student.agh.edu.pl/~grobelny/linux/BUG.tar
demonstrates the problem. I didn't play with compiler options myself but used
http://cvs.pld-linux.org/cgi-bin/cvsweb.cgi/SPECS/kernel-vanilla.spec (rev.
1.129) as basis for my build.
--
Regards,
Tomasz Grobelny