Date: Sun, 8 Dec 2002 17:07:56 +0300
From: Yar Tikhiy <yar@FreeBSD.ORG>
To: "Kenneth D. Merry" <ken@kdm.org>
Cc: Nate Lawson <nate@root.org>, freebsd-scsi@FreeBSD.ORG
Subject: Re: {da,sa,...}open bug?
Message-ID: <20021208170756.A75509@comp.chem.msu.su>
In-Reply-To: <20021206123401.A23249@panzer.kdm.org>; from ken@kdm.org on Fri, Dec 06, 2002 at 12:34:01PM -0700
References: <20021206145942.I80257@comp.chem.msu.su> <Pine.BSF.4.21.0212061121290.15885-100000@root.org> <20021206123401.A23249@panzer.kdm.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Dec 06, 2002 at 12:34:01PM -0700, Kenneth D. Merry wrote:
>
> Yar asked me to review his previous patch to a number of peripheral drivers
> that modifed the acquire/unlock/etc. order. I've asked Justin to take a
> look at it as well (thus the reason I haven't completed the review yet),
> but what you have above will likely cause a panic.
>
> cam_periph_acquire() only returns an error when periph == NULL. If, for
> some wild reason, a locked periph manages to get freed (thus causing
> cam_periph_acquire() to return CAM_REQ_CMP_ERR), cam_periph_unlock()
> will panic (NULL pointer deference) when called with a NULL periph.
First, I'd like to point out that freeing a periph doesn't cause a
pointer to it to become NULL. Therefore, cam_periph_unlock() is
extremely unlikely to panic since cam_periph_lock(), which in turn
calls cam_periph_acquire(), has been successful (i.e., periph != NULL)
My general impression is that that NULL check at the beginning of
cam_periph_acquire() is one of safety belts for unwary programmers
still left out there. I think it should be changed to something
like this:
KASSERT(periph != NULL, ("cam_periph_acquire: null periph"));
After this change, it will be OK to unlock a periph on cam_periph_acquire()
failure (granted someone have added code able to fail to the latter
function.)
--
Yar
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-scsi" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021208170756.A75509>