Skip to contentSkip to conversation

: Fix device leaks in mirred action

PATCH In review Patrick McHardy 2 participants 4 messages Nov 4, 2004

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 : Fix device leaks in mirred action

When a mirred action is created it takes two references to the device (dev_get_by_index + dev_hold), but only drops one when it gets destroyed. It also leaks a reference when a mirred action is replaced.

# This is a BitKeeper generated diff -Nru style patch. #
# ChangeSet
# 2004/11/04 07:28:30+01:00 [email protected]
# [PKT_SCHED]: Fix device leaks in mirred action
#
# Signed-off-by: Patrick McHardy <[email protected]>
#

# net/sched/mirred.c
#   2004/11/04 07:28:24+01:00 [email protected] +2 -1
#   [PKT_SCHED]: Fix device leaks in mirred action
#   
#   Signed-off-by: Patrick McHardy <[email protected]>
# 
diff -Nru a/net/sched/mirred.c b/net/sched/mirred.c
--- a/net/sched/mirred.c	2004-11-04 07:28:59 +01:00
+++ b/net/sched/mirred.c	2004-11-04 07:28:59 +01:00
@@ -138,8 +138,9 @@
 		p->eaction = parm->eaction;
 		if (parm->ifindex) {
 			p->ifindex = parm->ifindex;
+			if (ovr)
+				dev_put(p->dev);
 			p->dev = dev;
-			dev_hold(p->dev); 
 		}
 		spin_unlock(&p->lock);
 	}
#1 lore raw
Quoted 3 lines · Patrick McHardy

When a mirred action is created it takes two references to the device (dev_get_by_index + dev_hold), but only drops one when it gets destroyed.

Good catch.
The correct fix is to remove the dev_hold(p->dev) call.

Quoted 2 lines

It also leaks a reference when a mirred action
is replaced.

How? Is this patch supposed to fix that?

cheers,
jamal

#2 lore raw
Quoted 8 lines · jamal
Quoted 4 lines

It also leaks a reference when a mirred action
is replaced.

How? Is this patch supposed to fix that?

+ if (ovr)

+                               dev_put(p->dev);
                       p->dev = dev;

By dropping the reference before replacing it.

Regards
Patrick

#3 lore raw
Quoted 12 lines · Patrick McHardy
Quoted 6 lines · jamal
Quoted 2 lines

It also leaks a reference when a mirred action
is replaced.

How? Is this patch supposed to fix that?

(quoted diff) +2 -0
+                       if (ovr)
+                               dev_put(p->dev);
                        p->dev = dev;
Quoted 2 lines

By dropping the reference before replacing it.

Ok, makes sense. Thanks Patrick.
Dave please apply the patch in its entirety.

cheers,
jamal