Skip to contentSkip to conversation

cciss: unlock on error path

PATCH In review Dan Carpenter 3 participants 3 messages Apr 1, 2010 – Apr 2, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cciss: unlock on error path

We take the spin_lock again in fail_all_cmds() so we need to unlock here.

---
This was found by a static checker and I don't have the hardware myself. Please review carefully.

drivers/block/cciss.c +1 -0
index 9e3af30..eb5ff05 100644
@@ -3341,6 +3341,7 @@ static irqreturn_t do_cciss_intr(int irq, void *dev_id)
 					printk(KERN_WARNING
 					       "cciss: controller cciss%d failed, stopping.\n",
 					       h->ctlr);
+					spin_unlock_irqrestore(CCISS_LOCK(h->ctlr), flags);
 					fail_all_cmds(h->ctlr);
 					return IRQ_HANDLED;
 				}
#1 lore raw
Quoted 13 lines · Dan Carpenter

We take the spin_lock again in fail_all_cmds() so we need to unlock here.

---
This was found by a static checker and I don't have the hardware myself. Please review carefully.

drivers/block/cciss.c +0 -0
index 9e3af30..eb5ff05 100644

drivers/block/cciss.c static irqreturn_t do_cciss_intr(int irq, void *dev_id) · line 3347

Quoted 7 lines
drivers/block/cciss.c +1 -0
@@ -3341,6 +3341,7 @@ static irqreturn_t do_cciss_intr(int irq, void *dev_id)
 					printk(KERN_WARNING
 					       "cciss: controller cciss%d failed, stopping.\n",
 					       h->ctlr);
+					spin_unlock_irqrestore(CCISS_LOCK(h->ctlr), flags);
 					fail_all_cmds(h->ctlr);
 					return IRQ_HANDLED;
 				}

Looks ok to me. (Seems like I've seen this patch before). It's pretty much a paranoid "never happen" case anyway.
To hit this, the hardware would have to return a tag that was marked as containing an index, and then the index would have to be out of bounds, so either, it would have to be broken driver, broken firmware or broken hardware.

Really, one of these days, the interrupt handler should probably be cleaned up to be more like what's in the hpsa driver.

-- steve

#2 lore raw
In reply to Dan Carpenter
Quoted 2 lines · Dan Carpenter

We take the spin_lock again in fail_all_cmds() so we need to unlock here.

Thanks Dan, applied.

Signature · 1 line
Jens Axboe