From owner-freebsd-scsi Sun Dec 8 6: 8:11 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 B83DA37B401 for ; Sun, 8 Dec 2002 06:08:10 -0800 (PST) Received: from comp.chem.msu.su (comp-ext.chem.msu.su [158.250.32.157]) by mx1.FreeBSD.org (Postfix) with ESMTP id B41B543EBE for ; Sun, 8 Dec 2002 06:08:08 -0800 (PST) (envelope-from yar@comp.chem.msu.su) Received: from comp.chem.msu.su (localhost [127.0.0.1]) by comp.chem.msu.su (8.12.3/8.12.3) with ESMTP id gB8E83m2076045; Sun, 8 Dec 2002 17:08:03 +0300 (MSK) (envelope-from yar@comp.chem.msu.su) Received: (from yar@localhost) by comp.chem.msu.su (8.12.3/8.12.3/Submit) id gB8E7un8076044; Sun, 8 Dec 2002 17:07:56 +0300 (MSK) Date: Sun, 8 Dec 2002 17:07:56 +0300 From: Yar Tikhiy To: "Kenneth D. Merry" Cc: Nate Lawson , freebsd-scsi@FreeBSD.ORG Subject: Re: {da,sa,...}open bug? Message-ID: <20021208170756.A75509@comp.chem.msu.su> References: <20021206145942.I80257@comp.chem.msu.su> <20021206123401.A23249@panzer.kdm.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <20021206123401.A23249@panzer.kdm.org>; from ken@kdm.org on Fri, Dec 06, 2002 at 12:34:01PM -0700 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 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