Re: if_dc panics with 3Com OfficeConnect 10/100B PCI

On 2005-08-22 00:56, Giorgos Keramidas <keramida_at_freebsd.org> wrote:
> On 2005-08-16 13:48, John Baldwin <jhb_at_FreeBSD.org> wrote:
> >
> > Compile in DDB and get a stack trace over the serial console using 't' in DDB
> > instead of worrying about getting a dump.
>
> [...]
> I have no way to debug the crash now, since I only have one machine at
> home.  When I get the chance I plan to test this again, and I'll report
> back then, if this is still a problem.

I think I've finally located the cause of this panic.  The panic happens
early in dc_attach(), and the last message I see is the one printed by
dc_attach() in src/sys/pci/if_dc.c:

% 	if (sc->dc_res == NULL) {
% 		device_printf(dev, "couldn't map ports/memory\n");
% 		error = ENXIO;
% 		goto fail;
% 	}
%
% 	[ various busdma setup calls ]
%
% fail:
% 	if (error)
% 		dc_detach(dev);
% 	return (error);

The bug of dc_detach() that causes the panic is that it doesn't check if
one of the busdma tags and some maps it destroys have been allocated.  In
this case, because dc_attach() fails too early, before any busdma calls,
they haven't been allocated and one of the busdma calls panics when it gets
a NULL pointer.

The following patch fixes the panic of loading if_dc.ko when the 3Com
NIC of the subject is installed:

%%%
Index: if_dc.c
===================================================================
RCS file: /home/ncvs/src/sys/pci/if_dc.c,v
retrieving revision 1.166
diff -u -r1.166 if_dc.c
--- if_dc.c	29 Aug 2005 18:45:21 -0000	1.166
+++ if_dc.c	13 Sep 2005 00:32:51 -0000
_at__at_ -2364,11 +2364,17 _at__at_
 		bus_dmamem_free(sc->dc_stag, sc->dc_cdata.dc_sbuf, sc->dc_smap);
 	if (sc->dc_ldata != NULL)
 		bus_dmamem_free(sc->dc_ltag, sc->dc_ldata, sc->dc_lmap);
-	for (i = 0; i < DC_TX_LIST_CNT; i++)
-		bus_dmamap_destroy(sc->dc_mtag, sc->dc_cdata.dc_tx_map[i]);
-	for (i = 0; i < DC_RX_LIST_CNT; i++)
-		bus_dmamap_destroy(sc->dc_mtag, sc->dc_cdata.dc_rx_map[i]);
-	bus_dmamap_destroy(sc->dc_mtag, sc->dc_sparemap);
+	if (sc->dc_mtag) {
+		for (i = 0; i < DC_TX_LIST_CNT; i++)
+			if (sc->dc_cdata.dc_tx_map[i] != NULL)
+				bus_dmamap_destroy(sc->dc_mtag,
+				    sc->dc_cdata.dc_tx_map[i]);
+		for (i = 0; i < DC_RX_LIST_CNT; i++)
+			if (sc->dc_cdata.dc_rx_map[i] != NULL)
+				bus_dmamap_destroy(sc->dc_mtag,
+				    sc->dc_cdata.dc_rx_map[i]);
+		bus_dmamap_destroy(sc->dc_mtag, sc->dc_sparemap);
+	}
 	if (sc->dc_stag)
 		bus_dma_tag_destroy(sc->dc_stag);
 	if (sc->dc_mtag)
%%%

With this patch, on CURRENT of Sep-12-2005, the attach of the NIC in
question still fails, but at least it doesn't panic the system while
doing so :)

P.S.: Is there any way to guard against similar bugs in other drivers
and/or bugs that may be introduced later on?  For instance, would adding
KASSERT() macros in busdma_machdep.c slow things down too much?