linux-kernel - Re: [patch] cciss: unlock on error path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 1 Apr 2010 11:33:56 -0500
From:	scameron@...rdog.cce.hp.com
To:	Dan Carpenter <error27@...il.com>,
	Mike Miller <mike.miller@...com>,
	Jens Axboe <jens.axboe@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	iss_storagedev@...com, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [patch] cciss: unlock on error path

On Thu, Apr 01, 2010 at 06:54:04PM +0300, Dan Carpenter wrote:
> We take the spin_lock again in fail_all_cmds() so we need to unlock
> here.
> 
> Signed-off-by: Dan Carpenter <error27@...il.com>
> ---
> This was found by a static checker and I don't have the hardware myself.
> Please review carefully.
> 
> diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
> index 9e3af30..eb5ff05 100644
> --- a/drivers/block/cciss.c
> +++ b/drivers/block/cciss.c
> @@ -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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/