From: Jeff D. <jd...@ad...> - 2007-09-18 23:33:47
|
This patch fixes a crash caused by an interrupt coming in when an IRQ stack is being torn down. When this happens, handle_signal will loop, setting up the IRQ stack again because the tearing down had finished, and handling whatever signals had come in. However, to_irq_stack returns a mask of pending signals to be handled, plus bit zero is set if the IRQ stack was already active, and thus shouldn't be torn down. This causes a problem because when handle_signal goes around the loop, sig will be zero, and to_irq_stack will duly set bit zero in the returned mask, faking handle_signal into believing that it shouldn't tear down the IRQ stack and return thread_info pointers back to their original values. This will eventually cause a crash, as the IRQ stack thread_info will continue pointing to the original task_struct and an interrupt will look into it after it has been freed. The fix is to stop passing a signal number into to_irq_stack. Rather, the pending signals mask is initialized beforehand with the bit for sig already set. References to sig in to_irq_stack can be replaced with references to the mask. Signed-off-by: Jeff Dike <jd...@li...> --- arch/um/include/kern_util.h | 2 +- arch/um/kernel/irq.c | 7 ++++--- arch/um/os-Linux/signal.c | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) Index: linux-2.6.17/arch/um/include/kern_util.h =================================================================== --- linux-2.6.17.orig/arch/um/include/kern_util.h 2007-09-11 10:12:26.000000000 -0400 +++ linux-2.6.17/arch/um/include/kern_util.h 2007-09-18 12:31:28.000000000 -0400 @@ -117,7 +117,7 @@ extern void sigio_handler(int sig, union extern void copy_sc(union uml_pt_regs *regs, void *from); -unsigned long to_irq_stack(int sig, unsigned long *mask_out); +extern unsigned long to_irq_stack(unsigned long *mask_out); unsigned long from_irq_stack(int nested); #endif Index: linux-2.6.17/arch/um/kernel/irq.c =================================================================== --- linux-2.6.17.orig/arch/um/kernel/irq.c 2007-09-11 10:14:09.000000000 -0400 +++ linux-2.6.17/arch/um/kernel/irq.c 2007-09-18 12:32:08.000000000 -0400 @@ -518,13 +518,13 @@ int init_aio_irq(int irq, char *name, ir static unsigned long pending_mask; -unsigned long to_irq_stack(int sig, unsigned long *mask_out) +unsigned long to_irq_stack(unsigned long *mask_out) { struct thread_info *ti; unsigned long mask, old; int nested; - mask = xchg(&pending_mask, 1 << sig); + mask = xchg(&pending_mask, *mask_out); if(mask != 0){ /* If any interrupts come in at this point, we want to * make sure that their bits aren't lost by our @@ -534,7 +534,7 @@ unsigned long to_irq_stack(int sig, unsi * and pending_mask contains a bit for each interrupt * that came in. */ - old = 1 << sig; + old = *mask_out; do { old |= mask; mask = xchg(&pending_mask, old); @@ -550,6 +550,7 @@ unsigned long to_irq_stack(int sig, unsi task = cpu_tasks[ti->cpu].task; tti = task_thread_info(task); + *ti = *tti; ti->real_thread = tti; task->stack = ti; Index: linux-2.6.17/arch/um/os-Linux/signal.c =================================================================== --- linux-2.6.17.orig/arch/um/os-Linux/signal.c 2007-09-09 11:15:37.000000000 -0400 +++ linux-2.6.17/arch/um/os-Linux/signal.c 2007-09-18 12:32:40.000000000 -0400 @@ -119,7 +119,7 @@ void (*handlers[_NSIG])(int sig, struct void handle_signal(int sig, struct sigcontext *sc) { - unsigned long pending = 0; + unsigned long pending = 1 << sig; do { int nested, bail; @@ -134,7 +134,7 @@ void handle_signal(int sig, struct sigco * have to return, and the upper handler will deal * with this interrupt. */ - bail = to_irq_stack(sig, &pending); + bail = to_irq_stack(&pending); if(bail) return; |
From: Andrew M. <ak...@li...> - 2007-09-19 00:08:13
|
On Tue, 18 Sep 2007 19:33:36 -0400 Jeff Dike <jd...@ad...> wrote: > =================================================================== > --- linux-2.6.17.orig/arch/um/os-Linux/signal.c 2007-09-09 11:15:37.000000000 -0400 > +++ linux-2.6.17/arch/um/os-Linux/signal.c 2007-09-18 12:32:40.000000000 -0400 > @@ -119,7 +119,7 @@ void (*handlers[_NSIG])(int sig, struct > > void handle_signal(int sig, struct sigcontext *sc) > { > - unsigned long pending = 0; > + unsigned long pending = 1 << sig; You want 1UL there. |
From: Paolo G. <p.g...@gm...> - 2007-09-20 16:02:26
|
On mercoled=EC 19 settembre 2007, Andrew Morton wrote: > On Tue, 18 Sep 2007 19:33:36 -0400 > Jeff Dike <jd...@ad...> wrote: > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > --- linux-2.6.17.orig/arch/um/os-Linux/signal.c 2007-09-09 > > 11:15:37.000000000 -0400 +++ > > linux-2.6.17/arch/um/os-Linux/signal.c 2007-09-18 12:32:40.000000000 > > -0400 @@ -119,7 +119,7 @@ void (*handlers[_NSIG])(int sig, struct > > > > void handle_signal(int sig, struct sigcontext *sc) > > { > > - unsigned long pending =3D 0; > > + unsigned long pending =3D 1 << sig; > You want 1UL there. Yes, indeed - or sign extension on 64bit machines would set to 1 the whole= =20 high-word. But using long for that mask makes no difference; either int or long long (= or=20 better, either u32 or u64) should be used, given that the used signal range= =20 is the same on 32 and 64bit machines, it should be u32 for normal signals o= r=20 u64 if RT-signals are also allowed. =2D-=20 "Doh!" (cit.), I've made another mistake! Paolo Giarrusso, aka Blaisorblade |
From: Jeff D. <jd...@ad...> - 2007-09-24 20:02:14
|
On Thu, Sep 20, 2007 at 05:57:49PM +0200, Paolo Giarrusso wrote: > Yes, indeed - or sign extension on 64bit machines would set to 1 the whole > high-word. > > But using long for that mask makes no difference; either int or long > long (or better, either u32 or u64) should be used, given that the > used signal range is the same on 32 and 64bit machines, it should > be u32 for normal signals or u64 if RT-signals are also allowed. We don't use RT signals for anything, so we could use u32. I don't see it making much difference though. Jeff -- Work email - jdike at linux dot intel dot com |
From: Paolo G. <p.g...@gm...> - 2007-09-26 15:43:13
|
On luned=EC 24 settembre 2007, Jeff Dike wrote: > On Thu, Sep 20, 2007 at 05:57:49PM +0200, Paolo Giarrusso wrote: > > Yes, indeed - or sign extension on 64bit machines would set to 1 the > > whole high-word. > > > > But using long for that mask makes no difference; either int or long > > long (or better, either u32 or u64) should be used, given that the > > used signal range is the same on 32 and 64bit machines, it should > > be u32 for normal signals or u64 if RT-signals are also allowed. > > We don't use RT signals for anything, so we could use u32. I wasn't sure of that. > I don't=20 > see it making much difference though. Agreed - the only difference is for cleanliness and easy verification. =2D-=20 "Doh!" (cit.), I've made another mistake! Paolo Giarrusso, aka Blaisorblade |