Re: Race with netdevice unregister and dst_dev_event

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 28 Apr 2003 17:57:19 -0700 (PDT)
"David S. Miller" <davem@redhat.com> wrote:

>    From: Stephen Hemminger <shemminger@osdl.org>
>    Date: Mon, 28 Apr 2003 16:35:57 -0700
> 
>    So unless dst.c is fixed, there needs to be a rule NEVER use
>    dev->destructor in network modules.
> 
> I don't understand what the problem is.
> 
> Look, this code in the DST cache merely puts the device (which is
> legal from any context) if the device lacks a destructor.
> 
> If it has a destructor, it keeps the reference and once the DST entry
> dies the dst->dev is dev_put().  So it does in fact do the "cleanup"
> contrary to what you say, this happens during dst entry garbage
> collection.
> 
> If the dst has a dst->dev attached, the device has a reference
> and thus so does your module which created the device and thus
> your module cannot be unloaded.
> 
> So to reiterate, what exactly is the problem? :-)

The problem is that the destructor points to code that is in the module,
and it doesn't get run during unregister but later when the dst cache
GC timer expores. 
So it is possible for the module to go away before the DST entry is
released by dev_put() in the garbage collection (by timer). At that
point the destructor points to dead memory.

Possible workarounds:
	* don't use destructor in irda, vlan, bridge, ...
	  Go back to "old way".

	* make the cache smarter so when it receives a NETDEV_UNREGISTER
    	  message it purges right there, rather than waiting for the GC timer.

	* add a function to dev.c 
		netdev_kfreepriv(struct netdevice *dev) {
			 kfree(dev->priv);
		}
	  that would be safe since not part of the module.

	* Split device and private info into two structures.
	  Rather than having:
		struct xxx_info {
			...
			struct net_device dev;
			...
		};

		x = kmalloc();

		x->dev.priv = x;
		
	   xxx_destructor(struct net_device *dev) { kfree(dev->priv); }

	   Do:
		dev = kmalloc(sizeof(struct net_device))
		dev->priv = kmalloc(sizeof(struct xxx_info));
		dev->destructor = kfree;
	
	   unregistering...
		kfree(dev->priv);
		dev->priv = NULL;
		
           
-
: send the line "unsubscribe linux-net" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux 802.1Q VLAN]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Git]     [Bugtraq]     [Yosemite News and Information]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux PCI]     [Linux Admin]     [Samba]

  Powered by Linux