From 4db36a2b1c8eb532084746b89e2ba91fb0e7e5b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Tue, 1 Mar 2016 20:15:44 +0100 Subject: [PATCH] genirq: Validate action before dereferencing it in handle_irq_event_percpu() --- debian/changelog | 2 + ...action-before-dereferencing-it-in-ha.patch | 100 ++++++++++++++++++ debian/patches/series | 1 + 3 files changed, 103 insertions(+) create mode 100644 debian/patches/bugfix/all/genirq-Validate-action-before-dereferencing-it-in-ha.patch diff --git a/debian/changelog b/debian/changelog index 5f1c96106..97f9a1fff 100644 --- a/debian/changelog +++ b/debian/changelog @@ -162,6 +162,8 @@ linux (4.4.3-1) UNRELEASED; urgency=medium [ Uwe Kleine-König ] * [armhf] enable AXP20X_POWER (Closes: #815971) * [rt] Update to 4.4.3-rt9 + * genirq: Validate action before dereferencing it in + handle_irq_event_percpu() -- Salvatore Bonaccorso Sun, 28 Feb 2016 07:02:42 +0100 diff --git a/debian/patches/bugfix/all/genirq-Validate-action-before-dereferencing-it-in-ha.patch b/debian/patches/bugfix/all/genirq-Validate-action-before-dereferencing-it-in-ha.patch new file mode 100644 index 000000000..d12bd31e1 --- /dev/null +++ b/debian/patches/bugfix/all/genirq-Validate-action-before-dereferencing-it-in-ha.patch @@ -0,0 +1,100 @@ +From: Thomas Gleixner +Date: Wed, 13 Jan 2016 14:07:25 +0100 +Subject: [PATCH] genirq: Validate action before dereferencing it in + handle_irq_event_percpu() +Origin: v4.5-rc2, commit:570540d50710ed192e98e2f7f74578c9486b6b05 + +commit 71f64340fc0e changed the handling of irq_desc->action from + +CPU 0 CPU 1 +free_irq() lock(desc) + lock(desc) handle_edge_irq() + if (desc->action) { + handle_irq_event() + action = desc->action + unlock(desc) + desc->action = NULL handle_irq_event_percpu(desc, action) + action->xxx +to + +CPU 0 CPU 1 +free_irq() lock(desc) + lock(desc) handle_edge_irq() + if (desc->action) { + handle_irq_event() + unlock(desc) + desc->action = NULL handle_irq_event_percpu(desc, action) + action = desc->action + action->xxx + +So if free_irq manages to set the action to NULL between the unlock and before +the readout, we happily dereference a null pointer. + +We could simply revert 71f64340fc0e, but we want to preserve the better code +generation. A simple solution is to change the action loop from a do {} while +to a while {} loop. + +This is safe because we either see a valid desc->action or NULL. If the action +is about to be removed it is still valid as free_irq() is blocked on +synchronize_irq(). + +CPU 0 CPU 1 +free_irq() lock(desc) + lock(desc) handle_edge_irq() + handle_irq_event(desc) + set(INPROGRESS) + unlock(desc) + handle_irq_event_percpu(desc) + action = desc->action + desc->action = NULL while (action) { + action->xxx + ... + action = action->next; + sychronize_irq() + while(INPROGRESS); lock(desc) + clr(INPROGRESS) +free(action) + +That's basically the same mechanism as we have for shared +interrupts. action->next can become NULL while handle_irq_event_percpu() +runs. Either it sees the action or NULL. It does not matter, because action +itself cannot go away before the interrupt in progress flag has been cleared. + +Fixes: commit 71f64340fc0e "genirq: Remove the second parameter from handle_irq_event_percpu()" +Reported-by: zyjzyj2000@gmail.com +Signed-off-by: Thomas Gleixner +Cc: Huang Shijie +Cc: Jiang Liu +Cc: Peter Zijlstra +Cc: stable@vger.kernel.org +Link: http://lkml.kernel.org/r/alpine.DEB.2.11.1601131224190.3575@nanos +--- + kernel/irq/handle.c | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c +index a302cf9a2126..57bff7857e87 100644 +--- a/kernel/irq/handle.c ++++ b/kernel/irq/handle.c +@@ -138,7 +138,8 @@ irqreturn_t handle_irq_event_percpu(struct irq_desc *desc) + unsigned int flags = 0, irq = desc->irq_data.irq; + struct irqaction *action = desc->action; + +- do { ++ /* action might have become NULL since we dropped the lock */ ++ while (action) { + irqreturn_t res; + + trace_irq_handler_entry(irq, action); +@@ -173,7 +174,7 @@ irqreturn_t handle_irq_event_percpu(struct irq_desc *desc) + + retval |= res; + action = action->next; +- } while (action); ++ } + + add_interrupt_randomness(irq, flags); + +-- +2.7.0 + diff --git a/debian/patches/series b/debian/patches/series index e01b8bb21..0405482e6 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -133,6 +133,7 @@ bugfix/all/af_unix-guard-against-other-sk-in-unix_dgram_sendmsg.patch bugfix/all/revert-workqueue-make-sure-delayed-work-run-in-local-cpu.patch bugfix/all/af_unix-don-t-set-err-in-unix_stream_read_generic-unless-there-was-an-error.patch bugfix/all/bpf-fix-branch-offset-adjustment-on-backjumps-after-.patch +bugfix/all/genirq-Validate-action-before-dereferencing-it-in-ha.patch bugfix/x86/x86-mm-page-align-the-_end-symbol-to-avoid-pfn-conve.patch bugfix/x86/x86-mm-pat-ensure-cpa-pfn-only-contains-page-frame-n.patch bugfix/x86/x86-efi-map-ram-into-the-identity-page-table-for-mix.patch