From owner-freebsd-hackers@FreeBSD.ORG Sun Jan 18 10:45:04 2004 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 329B216A4CE; Sun, 18 Jan 2004 10:45:04 -0800 (PST) Received: from apollo.backplane.com (apollo.backplane.com [216.240.41.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0CAE743D2F; Sun, 18 Jan 2004 10:45:00 -0800 (PST) (envelope-from dillon@apollo.backplane.com) Received: from apollo.backplane.com (localhost [127.0.0.1]) i0IIiv82096390; Sun, 18 Jan 2004 10:44:58 -0800 (PST) (envelope-from dillon@apollo.backplane.com) Received: (from dillon@localhost) by apollo.backplane.com (8.12.9p2/8.12.9/Submit) id i0IIivlQ096389; Sun, 18 Jan 2004 10:44:57 -0800 (PST) (envelope-from dillon) Date: Sun, 18 Jan 2004 10:44:57 -0800 (PST) From: Matthew Dillon Message-Id: <200401181844.i0IIivlQ096389@apollo.backplane.com> To: Ruslan Ermilov References: <20040118160802.GC32115@FreeBSD.org.ua> cc: freebsd-hackers@freebsd.org cc: Paul Twohey cc: scsi@freebsd.org Subject: Re: [CHECKER] bugs in FreeBSD X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Jan 2004 18:45:04 -0000 These cam_sim_alloc() calls seem to be fairly critical to the operation of DPT and friends, why is it even possible for them to return NULL in the first place and what would be the effect of a (properly handled) NULL return if it did occur at this point? -Matt Matthew Dillon :> > * Create the device queue for our SIM. :> > */ :> > Start ---> :> > devq =3D cam_simq_alloc(dpt->max_dccbs); :> >=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-----