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: <20070320160217.GC31495@mellanox.co.il>
Date:	Tue, 20 Mar 2007 18:02:17 +0200
From:	"Michael S. Tsirkin" <mst@....mellanox.co.il>
To:	Alexey Kuznetsov <kuznet@....inr.ac.ru>
Cc:	"Michael S. Tsirkin" <mst@....mellanox.co.il>,
	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 Alexey Kuznetsov <kuznet@....inr.ac.ru>:
> Subject: Re: dst_ifdown breaks infiniband?
> 
> Hello!
> 
> > This might work. Could you post a patch to better show what you mean to do?
> 
> Here it is.
> 
> ->neigh_destructor() is killed (not used), replaced with ->neigh_cleanup(),
> which is called when neighbor entry goes to dead state. At this point
> everything is still valid: neigh->dev, neigh->parms etc.
> 
> The device should guarantee that dead neighbor entries (neigh->dead != 0)
> do not get private part initialized, otherwise nobody will cleanup it.

OK, I stress-tested this for about 9 hours - apparently this resolves the issues
I was seeing both with hotplug device unregister and module removal.

This is an old bug, but somehow it did not trigger on
older kernels - some code restructuring in infiniband is probably
the reason - so from that POV it's a regression in 2.6.21.
So now several people are experiencing these crashes.

David, Alexey, what do you think about this patch? Is it right?
Could this patch be considered for 2.6.21?

Acked-by: Michael S. Tsirkin <mst@....mellanox.co.il>

> 
> I think this is enough for ipoib which is the only user of this thing.
> Initialization private part of neighbor entries happens in ipib
> start_xmit routine, which is not reached when device is down.
> But it would be better to add explicit test for neigh->dead
> in any case.

Additionally, ip over infiniband actually tests a separate flag
IPOIB_FLAG_ADMIN_UP before looking at an skb.
This flag is cleared before the device goes down.
Taken together this should be sufficient I think.

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