Date: Fri, 31 Dec 2004 00:43:24 +0100 (CET) From: Antoine Brodin <antoine.brodin@laposte.net> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/75677: [patch] sk(4): plug leaks and fix acquiring duplicate lock of same type: "network driver" Message-ID: <200412302343.iBUNhOAD001307@barton.dreadbsd.org> Resent-Message-ID: <200412302350.iBUNo6bx033041@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 75677
>Category: kern
>Synopsis: [patch] sk(4): plug leaks and fix acquiring duplicate lock of same type: "network driver"
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Dec 30 23:50:06 GMT 2004
>Closed-Date:
>Last-Modified:
>Originator: Antoine Brodin
>Release: FreeBSD 6.0-CURRENT i386
>Organization:
none
>Environment:
System: FreeBSD barton.dreadbsd.org 6.0-CURRENT FreeBSD 6.0-CURRENT #1: Thu Dec 30 19:02:55 CET 2004 antoine@barton.dreadbsd.org:/usr/obj/usr/src/sys/BARTON i386
>Description:
* Witness reports this on a -current box :
acquiring duplicate lock of same type: "network driver"
1st nv0 @ /usr/ports/net/nvnet/work/nvnet/module/../src/if_nv.c:1511
2nd skc0 @ /usr/src/sys/modules/sk/../../pci/if_sk.c:1112
KDB: stack backtrace:
witness_checkorder(c2855c44,9,c07316ba,458) at witness_checkorder+0x500
_mtx_lock_flags(c2855c44,0,c07316ba,458,c2ac4e00) at _mtx_lock_flags+0x40
sk_jfree(e5a44340,c28e3000,0,c2857c00,e3c79c80) at sk_jfree+0x33
mb_free_ext(c2ac4e00) at mb_free_ext+0x36
m_freem(c2ac4e00,c2857c00,e5901000,e56fb0e0,c2857e60) at m_freem+0x21
nv_ospackettx(c2857c00,e56fb0e0,1,e5901000,0) at nv_ospackettx+0x73
UpdateTransmitDescRingData() at UpdateTransmitDescRingData+0xd3
I think this can cause a deadlock an a box with 2 sk(4) interfaces if
the 2 interfaces free a buffer allocated by the other at the same time.
A patch is attached (use an sk_jlist_mtx).
* When compiling if_sk as a module and unloading it, there seems to be
a few leaks.
A patch that fixes some of them is attached, but there is probably more
leaks.
Especially mii_phy_probe() and miibus_probe() allocate things during
a probe routine without freeing them and I suspect it's bad.
>How-To-Repeat:
* Enable witness on a box that acts as a gateway.
* Do kldload/kldunload if_sk and look at vmstat -m|grep devbuf.
>Fix:
--- sk.patch begins here ---
Index: if_sk.c
===================================================================
RCS file: /home/ncvs/src/sys/pci/if_sk.c,v
retrieving revision 1.94
diff -u -r1.94 if_sk.c
--- if_sk.c 24 Dec 2004 14:13:38 -0000 1.94
+++ if_sk.c 30 Dec 2004 21:43:06 -0000
@@ -1041,6 +1041,7 @@
SLIST_INIT(&sc_if->sk_jfree_listhead);
SLIST_INIT(&sc_if->sk_jinuse_listhead);
+ mtx_init(&sc_if->sk_jlist_mtx, "sk_jlist_mtx", NULL, MTX_DEF);
/*
* Now divide it up into 9K pieces and save the addresses
@@ -1053,8 +1054,15 @@
entry = malloc(sizeof(struct sk_jpool_entry),
M_DEVBUF, M_NOWAIT);
if (entry == NULL) {
- free(sc_if->sk_cdata.sk_jumbo_buf, M_DEVBUF);
+ while (!SLIST_EMPTY(&sc_if->sk_jfree_listhead)) {
+ entry = SLIST_FIRST(&sc_if->sk_jfree_listhead);
+ SLIST_REMOVE_HEAD(&sc_if->sk_jfree_listhead, jpool_entries);
+ free(entry, M_DEVBUF);
+ }
+ contigfree(sc_if->sk_cdata.sk_jumbo_buf, SK_JMEM,
+ M_DEVBUF);
sc_if->sk_cdata.sk_jumbo_buf = NULL;
+ mtx_destroy(&sc_if->sk_jlist_mtx);
printf("sk%d: no memory for jumbo "
"buffer queue!\n", sc_if->sk_unit);
return(ENOBUFS);
@@ -1076,7 +1084,7 @@
{
struct sk_jpool_entry *entry;
- SK_IF_LOCK_ASSERT(sc_if);
+ SK_JLIST_LOCK(sc_if);
entry = SLIST_FIRST(&sc_if->sk_jfree_listhead);
@@ -1089,6 +1097,9 @@
SLIST_REMOVE_HEAD(&sc_if->sk_jfree_listhead, jpool_entries);
SLIST_INSERT_HEAD(&sc_if->sk_jinuse_listhead, entry, jpool_entries);
+
+ SK_JLIST_UNLOCK(sc_if);
+
return(sc_if->sk_cdata.sk_jslots[entry->slot]);
}
@@ -1109,7 +1120,7 @@
if (sc_if == NULL)
panic("sk_jfree: didn't get softc pointer!");
- SK_IF_LOCK(sc_if);
+ SK_JLIST_LOCK(sc_if);
/* calculate the slot this buffer belongs to */
i = ((vm_offset_t)buf
@@ -1125,7 +1136,7 @@
SLIST_REMOVE_HEAD(&sc_if->sk_jinuse_listhead, jpool_entries);
SLIST_INSERT_HEAD(&sc_if->sk_jfree_listhead, entry, jpool_entries);
- SK_IF_UNLOCK(sc_if);
+ SK_JLIST_UNLOCK(sc_if);
return;
}
@@ -1737,8 +1748,21 @@
device_delete_child(dev, sc_if->sk_miibus);
*/
bus_generic_detach(dev);
- if (sc_if->sk_cdata.sk_jumbo_buf != NULL)
+ if (sc_if->sk_cdata.sk_jumbo_buf != NULL) {
+ struct sk_jpool_entry *entry;
+ while (!SLIST_EMPTY(&sc_if->sk_jfree_listhead)) {
+ entry = SLIST_FIRST(&sc_if->sk_jfree_listhead);
+ SLIST_REMOVE_HEAD(&sc_if->sk_jfree_listhead, jpool_entries);
+ free(entry, M_DEVBUF);
+ }
+ while (!SLIST_EMPTY(&sc_if->sk_jinuse_listhead)) {
+ entry = SLIST_FIRST(&sc_if->sk_jinuse_listhead);
+ SLIST_REMOVE_HEAD(&sc_if->sk_jinuse_listhead, jpool_entries);
+ free(entry, M_DEVBUF);
+ }
contigfree(sc_if->sk_cdata.sk_jumbo_buf, SK_JMEM, M_DEVBUF);
+ mtx_destroy(&sc_if->sk_jlist_mtx);
+ }
if (sc_if->sk_rdata != NULL) {
contigfree(sc_if->sk_rdata, sizeof(struct sk_ring_data),
M_DEVBUF);
Index: if_skreg.h
===================================================================
RCS file: /home/ncvs/src/sys/pci/if_skreg.h,v
retrieving revision 1.24
diff -u -r1.24 if_skreg.h
--- if_skreg.h 15 Nov 2004 19:37:21 -0000 1.24
+++ if_skreg.h 30 Dec 2004 17:42:22 -0000
@@ -1468,8 +1468,12 @@
int sk_if_flags;
SLIST_HEAD(__sk_jfreehead, sk_jpool_entry) sk_jfree_listhead;
SLIST_HEAD(__sk_jinusehead, sk_jpool_entry) sk_jinuse_listhead;
+ struct mtx sk_jlist_mtx;
};
+#define SK_JLIST_LOCK(_sc) mtx_lock(&(_sc)->sk_jlist_mtx)
+#define SK_JLIST_UNLOCK(_sc) mtx_unlock(&(_sc)->sk_jlist_mtx)
+
#define SK_MAXUNIT 256
#define SK_TIMEOUT 1000
#define ETHER_ALIGN 2
--- sk.patch ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200412302343.iBUNhOAD001307>