[PATCH 3.16 111/192] Input: i8042 - fix crash at boot time

2017-10-09 Thread Ben Hutchings 
3.16.49-rc1 review patch.  If anyone has any objections, please let me know.

--

From: Chen Hong 

commit 340d394a789518018f834ff70f7534fc463d3226 upstream.

The driver checks port->exists twice in i8042_interrupt(), first when
trying to assign temporary "serio" variable, and second time when deciding
whether it should call serio_interrupt(). The value of port->exists may
change between the 2 checks, and we may end up calling serio_interrupt()
with a NULL pointer:

BUG: unable to handle kernel NULL pointer dereference at 0050
IP: [] _spin_lock_irqsave+0x1f/0x40
PGD 0
Oops: 0002 [#1] SMP
last sysfs file:
CPU 0
Modules linked in:

Pid: 1, comm: swapper Not tainted 2.6.32-358.el6.x86_64 #1 QEMU Standard PC 
(i440FX + PIIX, 1996)
RIP: 0010:[]  [] 
_spin_lock_irqsave+0x1f/0x40
RSP: 0018:880028203cc0  EFLAGS: 00010082
RAX: 0001 RBX:  RCX: 
RDX: 0282 RSI: 0098 RDI: 0050
RBP: 880028203cc0 R08: 88013e79c000 R09: 880028203ee0
R10: 0298 R11: 0282 R12: 0050
R13:  R14:  R15: 0098
FS:  () GS:88002820() knlGS:
CS:  0010 DS: 0018 ES: 0018 CR0: 8005003b
CR2: 0050 CR3: 01a85000 CR4: 001407f0
DR0:  DR1:  DR2: 
DR3:  DR6: 0ff0 DR7: 0400
Process swapper (pid: 1, threadinfo 88013e79c000, task 88013e79b500)
Stack:
880028203d00 813de186 ff02 
    0098
 880028203d70 813e0162 880028203d20 8103b8ac
Call Trace:

 [] serio_interrupt+0x36/0xa0
[] i8042_interrupt+0x132/0x3a0
[] ? kvm_clock_read+0x1c/0x20
[] ? kvm_clock_get_cycles+0x9/0x10
[] handle_IRQ_event+0x60/0x170
[] ? kvm_guest_apic_eoi_write+0x44/0x50
[] handle_edge_irq+0xde/0x180
[] handle_irq+0x49/0xa0
[] do_IRQ+0x6c/0xf0
[] ret_from_intr+0x0/0x11
[] ? __do_softirq+0x73/0x1e0
[] ? hrtimer_interrupt+0x14b/0x260
[] ? call_softirq+0x1c/0x30
[] ? do_softirq+0x65/0xa0
[] ? irq_exit+0x85/0x90
[] ? smp_apic_timer_interrupt+0x70/0x9b
[] ? apic_timer_interrupt+0x13/0x20

To avoid the issue let's change the second check to test whether serio is
NULL or not.

Also, let's take i8042_lock in i8042_start() and i8042_stop() instead of
trying to be overly smart and using memory barriers.

Signed-off-by: Chen Hong 
[dtor: take lock in i8042_start()/i8042_stop()]
Signed-off-by: Dmitry Torokhov 
Signed-off-by: Ben Hutchings 
---
 drivers/input/serio/i8042.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -397,8 +397,10 @@ static int i8042_start(struct serio *ser
 {
struct i8042_port *port = serio->port_data;
 
+   spin_lock_irq(_lock);
port->exists = true;
-   mb();
+   spin_unlock_irq(_lock);
+
return 0;
 }
 
@@ -411,16 +413,20 @@ static void i8042_stop(struct serio *ser
 {
struct i8042_port *port = serio->port_data;
 
+   spin_lock_irq(_lock);
port->exists = false;
+   port->serio = NULL;
+   spin_unlock_irq(_lock);
 
/*
+* We need to make sure that interrupt handler finishes using
+* our serio port before we return from this function.
 * We synchronize with both AUX and KBD IRQs because there is
 * a (very unlikely) chance that AUX IRQ is raised for KBD port
 * and vice versa.
 */
synchronize_irq(I8042_AUX_IRQ);
synchronize_irq(I8042_KBD_IRQ);
-   port->serio = NULL;
 }
 
 /*
@@ -537,7 +543,7 @@ static irqreturn_t i8042_interrupt(int i
 
spin_unlock_irqrestore(_lock, flags);
 
-   if (likely(port->exists && !filtered))
+   if (likely(serio && !filtered))
serio_interrupt(serio, data, dfl);
 
  out:

[PATCH 3.16 111/192] Input: i8042 - fix crash at boot time

2017-10-09 Thread Ben Hutchings 
3.16.49-rc1 review patch.  If anyone has any objections, please let me know.

--

From: Chen Hong 

commit 340d394a789518018f834ff70f7534fc463d3226 upstream.

The driver checks port->exists twice in i8042_interrupt(), first when
trying to assign temporary "serio" variable, and second time when deciding
whether it should call serio_interrupt(). The value of port->exists may
change between the 2 checks, and we may end up calling serio_interrupt()
with a NULL pointer:

BUG: unable to handle kernel NULL pointer dereference at 0050
IP: [] _spin_lock_irqsave+0x1f/0x40
PGD 0
Oops: 0002 [#1] SMP
last sysfs file:
CPU 0
Modules linked in:

Pid: 1, comm: swapper Not tainted 2.6.32-358.el6.x86_64 #1 QEMU Standard PC 
(i440FX + PIIX, 1996)
RIP: 0010:[]  [] 
_spin_lock_irqsave+0x1f/0x40
RSP: 0018:880028203cc0  EFLAGS: 00010082
RAX: 0001 RBX:  RCX: 
RDX: 0282 RSI: 0098 RDI: 0050
RBP: 880028203cc0 R08: 88013e79c000 R09: 880028203ee0
R10: 0298 R11: 0282 R12: 0050
R13:  R14:  R15: 0098
FS:  () GS:88002820() knlGS:
CS:  0010 DS: 0018 ES: 0018 CR0: 8005003b
CR2: 0050 CR3: 01a85000 CR4: 001407f0
DR0:  DR1:  DR2: 
DR3:  DR6: 0ff0 DR7: 0400
Process swapper (pid: 1, threadinfo 88013e79c000, task 88013e79b500)
Stack:
880028203d00 813de186 ff02 
    0098
 880028203d70 813e0162 880028203d20 8103b8ac
Call Trace:

 [] serio_interrupt+0x36/0xa0
[] i8042_interrupt+0x132/0x3a0
[] ? kvm_clock_read+0x1c/0x20
[] ? kvm_clock_get_cycles+0x9/0x10
[] handle_IRQ_event+0x60/0x170
[] ? kvm_guest_apic_eoi_write+0x44/0x50
[] handle_edge_irq+0xde/0x180
[] handle_irq+0x49/0xa0
[] do_IRQ+0x6c/0xf0
[] ret_from_intr+0x0/0x11
[] ? __do_softirq+0x73/0x1e0
[] ? hrtimer_interrupt+0x14b/0x260
[] ? call_softirq+0x1c/0x30
[] ? do_softirq+0x65/0xa0
[] ? irq_exit+0x85/0x90
[] ? smp_apic_timer_interrupt+0x70/0x9b
[] ? apic_timer_interrupt+0x13/0x20

To avoid the issue let's change the second check to test whether serio is
NULL or not.

Also, let's take i8042_lock in i8042_start() and i8042_stop() instead of
trying to be overly smart and using memory barriers.

Signed-off-by: Chen Hong 
[dtor: take lock in i8042_start()/i8042_stop()]
Signed-off-by: Dmitry Torokhov 
Signed-off-by: Ben Hutchings 
---
 drivers/input/serio/i8042.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -397,8 +397,10 @@ static int i8042_start(struct serio *ser
 {
struct i8042_port *port = serio->port_data;
 
+   spin_lock_irq(_lock);
port->exists = true;
-   mb();
+   spin_unlock_irq(_lock);
+
return 0;
 }
 
@@ -411,16 +413,20 @@ static void i8042_stop(struct serio *ser
 {
struct i8042_port *port = serio->port_data;
 
+   spin_lock_irq(_lock);
port->exists = false;
+   port->serio = NULL;
+   spin_unlock_irq(_lock);
 
/*
+* We need to make sure that interrupt handler finishes using
+* our serio port before we return from this function.
 * We synchronize with both AUX and KBD IRQs because there is
 * a (very unlikely) chance that AUX IRQ is raised for KBD port
 * and vice versa.
 */
synchronize_irq(I8042_AUX_IRQ);
synchronize_irq(I8042_KBD_IRQ);
-   port->serio = NULL;
 }
 
 /*
@@ -537,7 +543,7 @@ static irqreturn_t i8042_interrupt(int i
 
spin_unlock_irqrestore(_lock, flags);
 
-   if (likely(port->exists && !filtered))
+   if (likely(serio && !filtered))
serio_interrupt(serio, data, dfl);
 
  out: