[patch] sti() preemption fix, 2.5.22

[patch] sti() preemption fix, 2.5.22

[Ingo Molnar]

__global_sti() appears to have a similar bug as __global_cli(), but it's a
bit more complex to trigger:

if a syscall-level, irqs-enabled process does a sti() but it does not hold
the IRQ spinlock, and the process gets preempted after the 'cpu'
assignment and gets run on another CPU, and the original CPU runs another
process that does a cli() which holds the global IRQ lock, then the rest
of __global_sti() will incorrectly release the IRQ lock - possibly causing
an oops in the other process.

this too is a few instructions race but looks quite serious at first
sight.

(the fix is to disable preemption around the critical section.)

	Ingo

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.510   -> 1.511  
#	arch/i386/kernel/irq.c	1.10    -> 1.11   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/06/17	mingo@elte.hu	1.511
# - sti() preemption bugfix.
# --------------------------------------------
#
diff -Nru a/arch/i386/kernel/irq.c b/arch/i386/kernel/irq.c
--- a/arch/i386/kernel/irq.c	Mon Jun 17 17:00:30 2002
+++ b/arch/i386/kernel/irq.c	Mon Jun 17 17:00:30 2002
@@ -368,9 +368,11 @@
 {
 	int cpu = smp_processor_id();
 
+	preempt_disable();
 	if (!local_irq_count(cpu))
 		release_irqlock(cpu);
 	__sti();
+	preempt_enable();
 }
 
 /*

Re: [patch] sti() preemption fix, 2.5.22

[Ingo Molnar]

Zoltan Szalontai noticed that my patch doesnt match the bugfix described
:-)

correct patch attached.

	Ingo

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.510   -> 1.511  
#	arch/i386/kernel/irq.c	1.10    -> 1.11   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/06/17	mingo@elte.hu	1.511
# - sti() preemption fix. for real.
# --------------------------------------------
#
diff -Nru a/arch/i386/kernel/irq.c b/arch/i386/kernel/irq.c
--- a/arch/i386/kernel/irq.c	Mon Jun 17 18:15:00 2002
+++ b/arch/i386/kernel/irq.c	Mon Jun 17 18:15:00 2002
@@ -366,11 +366,15 @@
 
 void __global_sti(void)
 {
-	int cpu = smp_processor_id();
+	int cpu;
+
+	preempt_disable();
+	cpu = smp_processor_id();
 
 	if (!local_irq_count(cpu))
 		release_irqlock(cpu);
 	__sti();
+	preempt_enable();
 }
 
 /*

Re: [patch] sti() preemption fix, 2.5.22

[Linus Torvalds]

On Mon, 17 Jun 2002, Ingo Molnar wrote:
>
> correct patch attached.

Ingo, please use "get_cpu()/put_cpu()" instead, which does exactly the
preempt-disable etc, and is more readable.

		Linus

Re: [patch] sti() preemption fix, 2.5.22

[Ingo Molnar]

On Mon, 17 Jun 2002, Linus Torvalds wrote:

> > correct patch attached.
> 
> Ingo, please use "get_cpu()/put_cpu()" instead, which does exactly the
> preempt-disable etc, and is more readable.

done, attached. (and pushed to my BK tree.)

(the cli() fix is special, there we can take advantage of the 'free' cli.)

	Ingo

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.510   -> 1.511  
#	arch/i386/kernel/irq.c	1.10    -> 1.11   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/06/17	mingo@elte.hu	1.511
# - sti() preemption fix.
# --------------------------------------------
#
diff -Nru a/arch/i386/kernel/irq.c b/arch/i386/kernel/irq.c
--- a/arch/i386/kernel/irq.c	Mon Jun 17 19:09:15 2002
+++ b/arch/i386/kernel/irq.c	Mon Jun 17 19:09:15 2002
@@ -366,11 +366,12 @@
 
 void __global_sti(void)
 {
-	int cpu = smp_processor_id();
+	int cpu = get_cpu();
 
 	if (!local_irq_count(cpu))
 		release_irqlock(cpu);
 	__sti();
+	put_cpu();
 }
 
 /*

Re: [patch] sti() preemption fix, 2.5.22

[Robert Love]

On Mon, 2002-06-17 at 10:04, Linus Torvalds wrote:

> Ingo, please use "get_cpu()/put_cpu()" instead, which does exactly the
> preempt-disable etc, and is more readable.

Here is Ingo's patch using the get_cpu()/put_cpu() interface where
needed...

Ingo, good catch!

	Robert Love

diff -urN linux-2.5.22/arch/i386/kernel/irq.c linux/arch/i386/kernel/irq.c
--- linux-2.5.22/arch/i386/kernel/irq.c	Sun Jun 16 19:31:24 2002
+++ linux/arch/i386/kernel/irq.c	Mon Jun 17 10:19:18 2002
@@ -356,8 +356,9 @@
 
 	__save_flags(flags);
 	if (flags & (1 << EFLAGS_IF_SHIFT)) {
-		int cpu = smp_processor_id();
+		int cpu;
 		__cli();
+		cpu = smp_processor_id();
 		if (!local_irq_count(cpu))
 			get_irqlock(cpu);
 	}
@@ -365,11 +366,15 @@
 
 void __global_sti(void)
 {
-	int cpu = smp_processor_id();
+	int cpu;
+
+	cpu = get_cpu();
 
 	if (!local_irq_count(cpu))
 		release_irqlock(cpu);
 	__sti();
+
+	put_cpu();
 }
 
 /*