netdev - Re: [PATCH 0/5] Call netif_carrier_off() after register

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: <1344984477.2690.85.camel@bwh-desktop.uk.solarflarecom.com>
Date:	Tue, 14 Aug 2012 23:47:57 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	David Miller <davem@...emloft.net>
CC:	<shchepetkov@...ras.ru>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <ldv-project@...ras.ru>
Subject: Re: [PATCH 0/5] Call netif_carrier_off() after register_netdev()

On Tue, 2012-08-14 at 14:00 -0700, David Miller wrote:
> From: Ilya Shchepetkov <shchepetkov@...ras.ru>
> Date: Tue, 14 Aug 2012 14:28:50 +0400
> 
> > Hi,
> > 
> > There are several patches on the subject:
> > 	
> > 	31bde1ceaa873bcaecd49e829bfabceacc4c512d
> > 	c55ad8e56b983f03589b38b4504b5d1f41161ff8
> > 	e826eafa65c6f1f7c8db5a237556cebac57ebcc5
> > 	0d672e9f8ac320c6d1ea9103db6df7f99ea20361
> > 	6a3c869a6021f4abcd69aa5fbb15c63f69eb36fe
> > 
> > In 2008, David Miller wrote in his commit:
> > (b47300168e770b60ab96c8924854c3b0eb4260eb)
> > 
> >>net: Do not fire linkwatch events until the device is registered.
> > 
> >>Several device drivers try to do things like netif_carrier_off()
> >>before register_netdev() is invoked.  This is bogus, but too many
> >>drivers do this to fix them all up in one go.
> > 
> > But I don't understand what will happen in this case?
> 
> Sigh... I would strongly suggest that when you don't understand
> something you leave it alone until you do.
> 
> You can't do the netif_carrier_off() after the device register because
> at the precise moment the device is registered it can be openned in
> parallel on another cpu and thus cause the entire carrier state
> to be changed.
> 
> Therefore if you do the netif_carrier_off() afterwards, it might
> be overwriting state changes made in another context.
> 
> Please just leave this code alone.

But if you do it beforehand then it doesn't have the intended effect.
(Supposed to be fixed by 22604c866889c4b2e12b73cbf1683bda1b72a313, which
had to be reverted: c276e098d3ee33059b4a1c747354226cec58487c.)

So you have to do it after, but without dropping the RTNL lock in
between.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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