Subject: kern/26011: pf leaks mbufs on disallowed packets
To: None <gnats-bugs@gnats.NetBSD.org>
From: Peter Postma <peter@pointless.nl>
List: netbsd-bugs
Date: 06/22/2004 18:59:37
>Number:         26011
>Category:       kern
>Synopsis:       pf leaks mbufs on disallowed packets
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jun 22 17:00:01 UTC 2004
>Closed-Date:
>Last-Modified:
>Originator:     Peter Postma
>Release:        NetBSD 2.0F
>Organization:
>Environment:
System: NetBSD mercury.pointless.nl 2.0F NetBSD 2.0F (mercury) #2: Tue Jun 22 18:43:56 CEST 2004 peter@mercury.pointless.nl:/usr/obj/sys/arch/sparc64/compile/mercury sparc64
Architecture: sparc64
Machine: sparc64
>Description:
i noticed this while i was doing my pf port, packets that
aren't allowed by pf are never freed. OpenBSD frees the mbuf
in ip_input.c/ip_output.c, after pf_test, but NetBSD doesn't
do this with pfil. So mbufs must be freed earlier, the pfil
wrapper functions would probably be the best choice.

>How-To-Repeat:
block packets with pf, netstat -m, see the mbufs growing.

>Fix:
Index: pf_ioctl.c
===================================================================
RCS file: /cvsroot/src/sys/dist/pf/net/pf_ioctl.c,v
retrieving revision 1.2
diff -u -r1.2 pf_ioctl.c
--- pf_ioctl.c	22 Jun 2004 14:17:07 -0000	1.2
+++ pf_ioctl.c	22 Jun 2004 16:41:34 -0000
@@ -2749,9 +2749,13 @@
 		}
 	}
 
-	if (pf_test(dir == PFIL_OUT ? PF_OUT : PF_IN, ifp, mp) != PF_PASS)
+	if (pf_test(dir == PFIL_OUT ? PF_OUT : PF_IN, ifp, mp) != PF_PASS) {
+		if (*mp != NULL) {
+			m_freem(*mp);
+			*mp = NULL;
+		}
 		return EHOSTUNREACH;
-	else
+	} else
 		return (0);
 }
 
@@ -2759,9 +2763,13 @@
 pfil6_wrapper(void *arg, struct mbuf **mp, struct ifnet *ifp, int dir)
 {
 
-	if (pf_test6(dir == PFIL_OUT ? PF_OUT : PF_IN, ifp, mp) != PF_PASS)
+	if (pf_test6(dir == PFIL_OUT ? PF_OUT : PF_IN, ifp, mp) != PF_PASS) {
+		if (*mp != NULL) {
+			m_freem(*mp);
+			*mp = NULL;
+		}
 		return EHOSTUNREACH;
-	else
+	} else
 		return (0);
 }
 
>Release-Note:
>Audit-Trail:
>Unformatted: