Date: Fri, 6 Dec 2002 12:34:01 -0700
From: "Kenneth D. Merry" <ken@kdm.org>
To: Nate Lawson <nate@root.org>
Cc: Yar Tikhiy <yar@FreeBSD.ORG>, freebsd-scsi@FreeBSD.ORG
Subject: Re: {da,sa,...}open bug?
Message-ID: <20021206123401.A23249@panzer.kdm.org>
In-Reply-To: <Pine.BSF.4.21.0212061121290.15885-100000@root.org>; from nate@root.org on Fri, Dec 06, 2002 at 11:23:56AM -0800
References: <20021206145942.I80257@comp.chem.msu.su> <Pine.BSF.4.21.0212061121290.15885-100000@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Dec 06, 2002 at 11:23:56 -0800, Nate Lawson wrote:
> On Fri, 6 Dec 2002, Yar Tikhiy wrote:
> > Yet another daopen() issue came to my attention. The DA_OPEN_FLAG bit
> > won't be reset back to 0 if daopen() has failed, thus leaving the device
> > marked as open. Isn't the below patch necessary?
> >
> > The rest of scsi_* modules seem to not have this bug.
> >
> > --
> > Yar
> >
> > Index: scsi_da.c
> > ===================================================================
> > RCS file: /home/ncvs/src/sys/cam/scsi/scsi_da.c,v
> > retrieving revision 1.116
> > diff -u -r1.116 scsi_da.c
> > --- scsi_da.c 29 Nov 2002 15:40:10 -0000 1.116
> > +++ scsi_da.c 6 Dec 2002 11:49:57 -0000
> > @@ -612,6 +612,7 @@
> > if ((softc->flags & DA_FLAG_PACK_REMOVABLE) != 0)
> > daprevent(periph, PR_PREVENT);
> > } else {
> > + softc->flags &= ~DA_FLAG_OPEN;
> > cam_periph_release(periph);
> > }
> > cam_periph_unlock(periph);
>
> You are correct. Ok to commit this w/ re@ approval The other periphs
> don't use this flag.
>
> There is another important fix that should be made that exists in several
> other drivers as well:
>
> --- scsi_da.c.orig Fri Dec 6 11:07:33 2002
> +++ scsi_da.c Fri Dec 6 11:10:44 2002
> @@ -545,8 +545,10 @@
> if ((error = cam_periph_lock(periph, PRIBIO|PCATCH)) != 0)
> return (error); /* error code from tsleep */
>
> - if (cam_periph_acquire(periph) != CAM_REQ_CMP)
> + if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
> + cam_periph_unlock(periph);
> return(ENXIO);
> + }
> softc->flags |= DA_FLAG_OPEN;
>
> if ((softc->flags & DA_FLAG_PACK_INVALID) != 0) {
>
> i.e. unlock if locked on return. Can you put together a patch for all of
> these?
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.
Ken
--
Kenneth Merry
ken@kdm.org
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?20021206123401.A23249>