netdev - Re: dst_ifdown breaks infiniband?

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: <20070319095545.GF8386@mellanox.co.il>
Date:	Mon, 19 Mar 2007 11:55:45 +0200
From:	"Michael S. Tsirkin" <mst@....mellanox.co.il>
To:	"Michael S. Tsirkin" <mst@....mellanox.co.il>
Cc:	Alexey Kuznetsov <kuznet@....inr.ac.ru>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	netdev@...r.kernel.org, general@...ts.openfabrics.org,
	Roland Dreier <rolandd@...co.com>,
	David Miller <davem@...emloft.net>
Subject: Re: dst_ifdown breaks infiniband?

> Quoting Michael S. Tsirkin <mst@....mellanox.co.il>:
> Subject: Re: dst_ifdown breaks infiniband?
> 
> > > Any simpler ideas?
> > 
> > Well, if inifiniband destructor really needs to take that lock... no.
> > Right now I do not see.
> 
> OK, this is actually not hard to fix - for infiniband, we can just look at
> neighbour->dev->type or compare neighbour->dev and
> neighbour->parms->dev - if they are different, device is being unregistered,
> so we do not need to do anything in the destructor.
> 
> I'll send a patch to openfabrics, shortly.
> 
> However, after implementing this fix, I hit what could be use after
> free at module unloading. Dave, Alexey, Roland, could you take a look at
> the following please?
> 
> Works fine for me (survived a couple of hours of crazy device
> loading/unloading/up/down/hotplug + link data and state activity)
> and seems to fix the issue.
> 
> ---------
> 
> If a device driver sets neigh_destructor in neigh_params, this could
> get called after the device has been unregistered and the driver module
> removed.
> 
> This is an old bug, but apparently, started to get triggered more infiniband
> after recent multicast and connected mode changes.
> 
> Fix this by delaying dev_put until the neigh_params object is removed.
> 
> Signed-off-by: Michael S. Tsirkin <mst@....mellanox.co.il>

The problem seems real enough but the fix seems no good -
device unregister gets blocked with
	unregister_netdevice: waiting for ib0 to become free. Usage count = 1

It seems the parms object can survive indefinitely after device is removed.

How about creating a new parms object in dst_ifdown, and pointing neighbour
to this?

Would that work?
The advantage of this approach is that neigh->parms is already protected
by RCU.

-- 
MST
-
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