netdev - Re: [PATCH] ipv4: remove all rt cache entries on UNREGISTER event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1285692969.3154.86.camel@edumazet-laptop>
Date:	Tue, 28 Sep 2010 18:56:09 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	nicolas.dichtel@...nd.com
Cc:	netdev <netdev@...r.kernel.org>,
	Octavian Purdila <opurdila@...acom.com>
Subject: Re: [PATCH] ipv4: remove all rt cache entries on UNREGISTER event

Le mardi 28 septembre 2010 à 18:45 +0200, Nicolas Dichtel a écrit :
> Eric Dumazet wrote:
> > Le mardi 28 septembre 2010 à 17:24 +0200, Nicolas Dichtel a écrit :
> >> Hi,
> >>
> >> I face a problem when I try to remove an interface, 
> >> netdev_wait_allrefs() complains about refcount.
> >>
> >> Here is a trivial scenario to reproduce the problem:
> >> # ip tunnel add mode ipip remote 10.16.0.164 local 10.16.0.72 dev eth0
> >> # ./a.out tunl1
> >> # ip tunnel del tunl1
> >>
> >> Note: a.out binary create an IPv4 raw socket, attach it to tunl1 
> >> (SO_BINDTODEVICE), set it as multicast (IP_MULTICAST_LOOP), set the 
> >> multicast interface to tunl1 (IP_MULTICAST_IF), build the IP header 
> >> (IP_HDRINCL) and then send a single packet (192.168.6.1 -> 224.0.0.18).
> >>
> >> Note2: when a.out is executed, tunl1 has no ip address and is down.
> >>
> > 
> > CC Octavian Purdila, the patch author.
> > 
> > I am just wondering why this route is created in the first place.
> At first, I asked myself the same question, but it seems that this is 
> allowed to send a packet through this kind of socket, even if interface 
> is down. Packet will be destroyed by the noop qdisk.
> But I agree that it is strange to perform route lookup and everything to 
>    destroy the packet at the end ...
> Maybe raw_sendmsg() can delete it directly ;-) ... or maybe 
> ip_route_output_flow().
> 
> Any suggestions welcome.
> 

Hmm...

One way to track this kind of problem would be to add a WARN_ON() in
dev_hold()

-> Check that when a reference on dev is taken, we are in a known state.

Something like this ?

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 83de0eb..54bef78 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1773,6 +1774,7 @@ static inline void dev_put(struct net_device *dev)
  */
 static inline void dev_hold(struct net_device *dev)
 {
+	WARN_ON(dev->reg_state != NETREG_REGISTERED);
 	atomic_inc(&dev->refcnt);
 }
 


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html