Re: Mbuf leak in if_lagg.c

[Alexandre Martins]

Le jeudi 26 mars 2015, 23:10:49 Andrey V. Elsukov a écrit :
 On 26.03.2015 22:42, Andrey V. Elsukov wrote:
  If lp_detaching is non 0, the mbuf pointer is set to NULL without m_freem
  it.
  
  Can you look at this ?
  
  Hi,
  
  what you thing about this patch?
  lp_detaching can be non zero in case of parent interface departure.
  So I don't see the reason to call ETHER_BPF_MTAP() in this case.
 
 Now I see the reason - to capture all received packets before interface
 departure. New is attached.

Sounds good for me :)

-- 
Alexandre Martins
STORMSHIELD



smime.p7s
Description: S/MIME cryptographic signature

Re: Mbuf leak in if_lagg.c

[Adrian Chadd]

Hi!

Please file a bug! https://bugs.freebsd.org/submit/

Thanks!


-a


On 19 March 2015 at 09:31, Alexandre Martins
alexandre.mart...@stormshield.eu wrote:
 Hi !

 I found a leak of mbuf in the lagg driver :

 https://svnweb.freebsd.org/base/head/sys/net/if_lagg.c?view=annotate#l1672

 -=-=-=-=-=-=-=-=-=-=-
 m = (lp-lp_detaching == 0) ? lagg_proto_input(sc, lp, m) : NULL;
 -=-=-=-=-=-=-=-=-=-=-

 If lp_detaching is non 0, the mbuf pointer is set to NULL without m_freem it.

 Can you look at this ?

 Regards

 --
 Alexandre Martins
 STORMSHIELD

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: Mbuf leak in if_lagg.c

[Andrey V. Elsukov]

On 19.03.2015 19:31, Alexandre Martins wrote:
 Hi !
 
 I found a leak of mbuf in the lagg driver :
 
 https://svnweb.freebsd.org/base/head/sys/net/if_lagg.c?view=annotate#l1672
 
 -=-=-=-=-=-=-=-=-=-=-
 m = (lp-lp_detaching == 0) ? lagg_proto_input(sc, lp, m) : NULL;
 -=-=-=-=-=-=-=-=-=-=-
 
 If lp_detaching is non 0, the mbuf pointer is set to NULL without m_freem it.
 
 Can you look at this ?

Hi,

what you thing about this patch?
lp_detaching can be non zero in case of parent interface departure.
So I don't see the reason to call ETHER_BPF_MTAP() in this case.

-- 
WBR, Andrey V. Elsukov
Index: if_lagg.c
===
--- if_lagg.c	(revision 280234)
+++ if_lagg.c	(working copy)
@@ -1661,7 +1661,8 @@ lagg_input(struct ifnet *ifp, struct mbuf *m)
 	LAGG_RLOCK(sc, tracker);
 	if ((scifp-if_drv_flags  IFF_DRV_RUNNING) == 0 ||
 	(lp-lp_flags  LAGG_PORT_DISABLED) ||
-	sc-sc_proto == LAGG_PROTO_NONE) {
+	sc-sc_proto == LAGG_PROTO_NONE ||
+	lp-lp_detaching != 0) {
 		LAGG_RUNLOCK(sc, tracker);
 		m_freem(m);
 		return (NULL);
@@ -1668,9 +1669,7 @@ lagg_input(struct ifnet *ifp, struct mbuf *m)
 	}
 
 	ETHER_BPF_MTAP(scifp, m);
-
-	m = (lp-lp_detaching == 0) ? lagg_proto_input(sc, lp, m) : NULL;
-
+	m = lagg_proto_input(sc, lp, m);
 	if (m != NULL) {
 		if (scifp-if_flags  IFF_MONITOR) {
 			m_freem(m);


signature.asc
Description: OpenPGP digital signature

Re: Mbuf leak in if_lagg.c

[Andrey V. Elsukov]

On 26.03.2015 22:42, Andrey V. Elsukov wrote:
 If lp_detaching is non 0, the mbuf pointer is set to NULL without m_freem it.

 Can you look at this ?
 
 Hi,
 
 what you thing about this patch?
 lp_detaching can be non zero in case of parent interface departure.
 So I don't see the reason to call ETHER_BPF_MTAP() in this case.

Now I see the reason - to capture all received packets before interface
departure. New is attached.

-- 
WBR, Andrey V. Elsukov
Index: if_lagg.c
===
--- if_lagg.c	(revision 280234)
+++ if_lagg.c	(working copy)
@@ -1669,7 +1669,11 @@ lagg_input(struct ifnet *ifp, struct mbuf *m)
 
 	ETHER_BPF_MTAP(scifp, m);
 
-	m = (lp-lp_detaching == 0) ? lagg_proto_input(sc, lp, m) : NULL;
+	if (lp-lp_detaching != 0) {
+		m_freem(m);
+		m = NULL;
+	} else
+		m = lagg_proto_input(sc, lp, m);
 
 	if (m != NULL) {
 		if (scifp-if_flags  IFF_MONITOR) {


signature.asc
Description: OpenPGP digital signature