From owner-freebsd-scsi@FreeBSD.ORG Sun Jan 18 08:08:13 2004 Return-Path: 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 68CA016A4CE; Sun, 18 Jan 2004 08:08:13 -0800 (PST) Received: from phantom.cris.net (phantom.cris.net [212.110.130.74]) by mx1.FreeBSD.org (Postfix) with ESMTP id 56CD943D31; Sun, 18 Jan 2004 08:08:06 -0800 (PST) (envelope-from ru@FreeBSD.org.ua) Received: from phantom.cris.net (ru@localhost [127.0.0.1]) by phantom.cris.net (8.12.10/8.12.10) with ESMTP id i0IG86jm032701; Sun, 18 Jan 2004 18:08:09 +0200 (EET) (envelope-from ru@FreeBSD.org.ua) Received: (from ru@localhost) by phantom.cris.net (8.12.10/8.12.10/Submit) id i0IG834v032696; Sun, 18 Jan 2004 18:08:03 +0200 (EET) (envelope-from ru) Date: Sun, 18 Jan 2004 18:08:02 +0200 From: Ruslan Ermilov To: Paul Twohey Message-ID: <20040118160802.GC32115@FreeBSD.org.ua> References: <20040118154447.GA32115@FreeBSD.org.ua> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="SO98HVl1bnMOfKZd" Content-Disposition: inline In-Reply-To: <20040118154447.GA32115@FreeBSD.org.ua> User-Agent: Mutt/1.5.5.1i cc: freebsd-hackers@freebsd.org cc: scsi@freebsd.org Subject: Re: [CHECKER] bugs in FreeBSD X-BeenThere: freebsd-scsi@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: SCSI subsystem List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Jan 2004 16:08:13 -0000 --SO98HVl1bnMOfKZd Content-Type: multipart/mixed; boundary="yLVHuoLXiP9kZBkt" Content-Disposition: inline --yLVHuoLXiP9kZBkt Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Jan 18, 2004 at 05:44:48PM +0200, Ruslan Ermilov wrote: > On Fri, Jan 16, 2004 at 04:09:34PM -0800, Paul Twohey wrote: > [...] > > --------------------------------------------------------- > > [BUG] > > /u2/engler/mc/freebsd/sys/i386/compile/GENERIC/../../../dev/dpt/dpt_scs= i.c:1542:dpt_attach:ERROR:LEAK:1542:1571: pointer=3Ddevq from RO=3Dcam_simq= _alloc(-1) [s=3D21,pop=3D21,pr=3D0.99] [rank=3Dmed] leaked! [z=3D1.0] [succ= ess=3D3] > >=20 > > int i; > >=20 > > /* > > * Create the device queue for our SIM. > > */ > > Start ---> > > devq =3D cam_simq_alloc(dpt->max_dccbs); > >=20 > > ... DELETED 23 lines ... > >=20 > >=20 > > } > > if (i > 0) > > EVENTHANDLER_REGISTER(shutdown_final, dptshutdown, > > dpt, SHUTDOWN_PRI_DEFAULT); > > Error ---> > > return (i); > > } > >=20 > > int > > --------------------------------------------------------- >=20 > We aren't leaking "devq" here, it's freed (if necessary) by setting > the second cam_sim_free() argument to true: >=20 > if (xpt_bus_register(dpt->sims[i], i) !=3D CAM_SUCCESS) { > cam_sim_free(dpt->sims[i], /*free_devq*/i =3D=3D = 0); > break; > } >=20 > But we're missing the proper NULL checking, here's the fix: >=20 > %%% > Index: dpt_scsi.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /home/ncvs/src/sys/dev/dpt/dpt_scsi.c,v > retrieving revision 1.45 > diff -u -p -r1.45 dpt_scsi.c > --- dpt_scsi.c 24 Aug 2003 17:46:04 -0000 1.45 > +++ dpt_scsi.c 18 Jan 2004 15:39:13 -0000 > @@ -1553,6 +1553,8 @@ dpt_attach(dpt_softc_t *dpt) > dpt->sims[i] =3D cam_sim_alloc(dpt_action, dpt_poll, "dpt", > dpt, dpt->unit, /*untagged*/2, > /*tagged*/dpt->max_dccbs, devq); > + if (dpt->sims[i] =3D=3D NULL) > + break; > if (xpt_bus_register(dpt->sims[i], i) !=3D CAM_SUCCESS) { > cam_sim_free(dpt->sims[i], /*free_devq*/i =3D=3D 0); > break; > %%% >=20 Bah, but with this patch that avoids the NULL pointer dereference we start leaking devq. Attached is a more complete patch, and for dev/irr/irr.c too. Cheers, --=20 Ruslan Ermilov FreeBSD committer ru@FreeBSD.org --yLVHuoLXiP9kZBkt Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=p Index: dpt/dpt_scsi.c =================================================================== RCS file: /home/ncvs/src/sys/dev/dpt/dpt_scsi.c,v retrieving revision 1.45 diff -u -p -r1.45 dpt_scsi.c --- dpt/dpt_scsi.c 24 Aug 2003 17:46:04 -0000 1.45 +++ dpt/dpt_scsi.c 18 Jan 2004 15:51:44 -0000 @@ -1553,6 +1553,11 @@ dpt_attach(dpt_softc_t *dpt) dpt->sims[i] = cam_sim_alloc(dpt_action, dpt_poll, "dpt", dpt, dpt->unit, /*untagged*/2, /*tagged*/dpt->max_dccbs, devq); + if (dpt->sims[i] == NULL) { + if (i == 0) + cam_simq_free(devq); + break; + } if (xpt_bus_register(dpt->sims[i], i) != CAM_SUCCESS) { cam_sim_free(dpt->sims[i], /*free_devq*/i == 0); break; Index: iir/iir.c =================================================================== RCS file: /home/ncvs/src/sys/dev/iir/iir.c,v retrieving revision 1.9 diff -u -p -r1.9 iir.c --- iir/iir.c 26 Sep 2003 15:36:47 -0000 1.9 +++ iir/iir.c 18 Jan 2004 15:52:04 -0000 @@ -510,6 +510,11 @@ iir_attach(struct gdt_softc *gdt) gdt->sims[i] = cam_sim_alloc(iir_action, iir_poll, "iir", gdt, gdt->sc_hanum, /*untagged*/2, /*tagged*/GDT_MAXCMDS, devq); + if (gdt->sims[i] == NULL) { + if (i == 0) + cam_simq_free(devq); + break; + } if (xpt_bus_register(gdt->sims[i], i) != CAM_SUCCESS) { cam_sim_free(gdt->sims[i], /*free_devq*/i == 0); break; --yLVHuoLXiP9kZBkt-- --SO98HVl1bnMOfKZd Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.4 (FreeBSD) iD8DBQFACq9iUkv4P6juNwoRAghWAKCBpqGJmtW1g7vOJS15YgKfg/782QCeImr/ aZ5eUYh2kvOaSBl5zcFd4mE= =j+I+ -----END PGP SIGNATURE----- --SO98HVl1bnMOfKZd--