From owner-freebsd-scsi Fri Dec 6 11:34: 6 2002 Delivered-To: freebsd-scsi@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2E2B537B401; Fri, 6 Dec 2002 11:34:04 -0800 (PST) Received: from panzer.kdm.org (panzer.kdm.org [216.160.178.169]) by mx1.FreeBSD.org (Postfix) with ESMTP id 25DC443ED8; Fri, 6 Dec 2002 11:34:03 -0800 (PST) (envelope-from ken@panzer.kdm.org) Received: from panzer.kdm.org (localhost [127.0.0.1]) by panzer.kdm.org (8.12.6/8.12.5) with ESMTP id gB6JY2nU023317; Fri, 6 Dec 2002 12:34:02 -0700 (MST) (envelope-from ken@panzer.kdm.org) Received: (from ken@localhost) by panzer.kdm.org (8.12.6/8.12.5/Submit) id gB6JY2Cg023316; Fri, 6 Dec 2002 12:34:02 -0700 (MST) (envelope-from ken) Date: Fri, 6 Dec 2002 12:34:01 -0700 From: "Kenneth D. Merry" To: Nate Lawson Cc: Yar Tikhiy , freebsd-scsi@FreeBSD.ORG Subject: Re: {da,sa,...}open bug? Message-ID: <20021206123401.A23249@panzer.kdm.org> References: <20021206145942.I80257@comp.chem.msu.su> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: ; from nate@root.org on Fri, Dec 06, 2002 at 11:23:56AM -0800 Sender: owner-freebsd-scsi@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org 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