Messages in this thread Patch in this message | | | Subject | Re: [RFC] x86, fpu: unify signal handling code paths for x86 and x86_64 kernels | From | Suresh Siddha <> | Date | Thu, 14 Jun 2012 17:10:35 -0700 |
| |
On Thu, 2012-06-14 at 13:45 -0700, Suresh Siddha wrote: > On Thu, 2012-06-14 at 16:37 +0200, Hans Rosenfeld wrote: > > Thank you for working on this. > > > > I didn't see anything obviously wrong in a quick review of this code, > > but this stuff is too complex for this to mean anything :) > > > > I ran a quick test of your code. I found a signal frame corruption when > > running a 32bit test program on a 64bit kernel. I didn't try to find out > > why it failed, but I'll send you the test program in a private mail so > > you can look at it yourself. > > > > Ok. The problem was that I was using is_ia32_task() (which uses > TS_COMPAT) to check if the task is a compat task or not. And that flag > is not set during the signal delivery paths for the compat task. > > I guess I should be using TIF_IA32 check instead. Using TIF_IA32 check > makes your test case run fine. > > Will update it accordingly. >
For the above mentioned reason, I guess the current usage of is_ia32_task() in copy_siginfo_to_user32() (added recently for x32 support) is broken, as the TS_COMPAT flag may not be set for the x86 compat mode apps in those paths.
Peter offline suggested that the signal delivery path should probably set/clear the TS_COMPAT flag (just like we do it for syscall paths), so that is_ia32_task() will work in those paths.
But the exception paths for the 32bit and 64bit apps in the 64-bit kernel is same. So I really need to use something like TIF_IA32 to find out the compat mode of the task. Anyways, the comment in the below patch explains the problem ;) What should we do? Just remove the is_ia32_task() checks in signal paths and just use TIF_IA32 or do something like below.
My personal preference is to use TIF_IA32 check and avoid the usage of is_ia32_task() in the signal delivery paths.
Signal return goes through a system call which already sets the TS_COMPAT. It is the signal delivery that is causing the asymmetry. ---
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c index b280908..d7f01fc 100644 --- a/arch/x86/kernel/signal.c +++ b/arch/x86/kernel/signal.c @@ -791,9 +791,39 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags) } /* deal with pending signal delivery */ - if (thread_info_flags & _TIF_SIGPENDING) + if (thread_info_flags & _TIF_SIGPENDING) { + bool compat_flag = false; + + /* + * Check if the task is a compat task and we are here + * in a non-syscall path. If so, set TS_COMPAT explicitly + * so that do_signal() can use is_compat_task()/is_ia32_task(). + * + * Or Should we just use test_thread_flag(TIF_IA32) check + * (which is called just 'is_ia32' in this file) instead of + * using is_ia32_task() in signal delivery paths. + * + * BTW, ideally is_[ia32|x32|compat]_task() should be really + * called as is_[ia32|x32|compat]_syscall() and the + * TIF_IA32 check can then be called as is_ia32_task etc.. + * + * Sigh... + * + */ + if (is_ia32 && !is_ia32_task()) { + current_thread_info()->status |= TS_COMPAT; + compat_flag = true; + } + do_signal(regs); + /* + * Clear the TS_COMPAT only if we set it above. + */ + if (compat_flag) + current_thread_info()->status &= ~TS_COMPAT; + } + if (thread_info_flags & _TIF_NOTIFY_RESUME) { clear_thread_flag(TIF_NOTIFY_RESUME); tracehook_notify_resume(regs);
| |