possible mbuf leak in tap_dev_close()

[Paul Forgey]

In the NetBSD Current sources downloaded Nov 20, I noticed a potential mbuf leak in the tap driver if the character device is closed with unread packets.

$ diff -u if_tap.c~ if_tap.c
--- if_tap.c~ 2009-11-25 23:18:34.000000000 -0800
+++ if_tap.c 2009-11-25 23:21:47.000000000 -0800
@@ -839,6 +839,7 @@
{
struct ifnet *ifp;
int s;
+ struct mbuf *m;

s = splnet();
/* Let tap_start handle packets again */
@@ -846,20 +847,17 @@
ifp->if_flags &= ~IFF_OACTIVE;

/* Purge output queue */
- if (!(IFQ_IS_EMPTY(&ifp->if_snd))) {
- struct mbuf *m;
-
- for (;;) {
- IFQ_DEQUEUE(&ifp->if_snd, m);
- if (m == NULL)
- break;
+ for (;;) {
+ IFQ_DEQUEUE(&ifp->if_snd, m);
+ if (m == NULL)
+ break;

- ifp->if_opackets++;
+ ifp->if_opackets++;
#if NBPFILTER > 0
- if (ifp->if_bpf)
- bpf_mtap(ifp->if_bpf, m);
+ if (ifp->if_bpf)
+ bpf_mtap(ifp->if_bpf, m);
#endif
- }
+ m_free (m);
}
splx(s);


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Iain Hibbert]

Post by Paul Forgey
In the NetBSD Current sources downloaded Nov 20, I noticed a potential
mbuf leak in the tap driver if the character device is closed with
unread packets.

I have fixed it, thanks for the report.

- I did not remove the empty-list check, that could go but it is an
unrelated style change.

- Also, m_freem() should be used to release the chain as a packet is often
spread out over several mbufs

iain


Index: if_tap.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_tap.c,v
retrieving revision 1.59
diff -u -r1.59 if_tap.c
--- if_tap.c 15 Sep 2009 19:38:15 -0000 1.59
+++ if_tap.c 29 Nov 2009 10:17:04 -0000
@@ -859,6 +859,7 @@
if (ifp->if_bpf)
bpf_mtap(ifp->if_bpf, m);
#endif
+ m_freem(m);
}
}
splx(s);



--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de