[CHECKER] bugs in FreeBSD

[Ruslan Ermilov]

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_scsi.c:1542:dpt_attach:ERROR:LEAK:1542:1571: pointer=devq from RO=cam_simq_alloc(-1) [s=21,pop=21,pr=0.99] [rank=med] leaked! [z=1.0] [success=3]
> > 
> > 	int i;
> > 
> > 	/*
> > 	 * Create the device queue for our SIM.
> > 	 */
> > Start --->
> > 	devq = cam_simq_alloc(dpt->max_dccbs);
> > 
> > 	... DELETED 23 lines ...
> > 
> > 
> > 	}
> > 	if (i > 0)
> > 		EVENTHANDLER_REGISTER(shutdown_final, dptshutdown,
> > 				      dpt, SHUTDOWN_PRI_DEFAULT);
> > Error --->
> > 	return (i);
> > }
> > 
> > int
> > ---------------------------------------------------------
> 
> We aren't leaking "devq" here, it's freed (if necessary) by setting
> the second cam_sim_free() argument to true:
> 
>                 if (xpt_bus_register(dpt->sims[i], i) != CAM_SUCCESS) {
>                         cam_sim_free(dpt->sims[i], /*free_devq*/i == 0);
>                         break;
>                 }
> 
> But we're missing the proper NULL checking, here's the fix:
> 
> %%%
> Index: 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_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] = cam_sim_alloc(dpt_action, dpt_poll, "dpt",
>  					     dpt, dpt->unit, /*untagged*/2,
>  					     /*tagged*/dpt->max_dccbs, devq);
> +		if (dpt->sims[i] == NULL)
> +			break;
>  		if (xpt_bus_register(dpt->sims[i], i) != CAM_SUCCESS) {
>  			cam_sim_free(dpt->sims[i], /*free_devq*/i == 0);
>  			break;
> %%%
> 
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,
-- 
Ruslan Ermilov
FreeBSD committer
ru at FreeBSD.org
-------------- next part --------------
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;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-scsi/attachments/20040118/7b65f231/attachment.bin