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] [day] [month] [year] [list]

Message-ID: <20120817092002.191a8869@nehalam.linuxnetplumber.net>
Date:	Fri, 17 Aug 2012 09:20:02 -0700
From:	Stephen Hemminger <shemminger@...tta.com>
To:	Ilya Shchepetkov <shchepetkov@...ras.ru>
Cc:	<bjorn@...k.no>, "David S. Miller" <davem@...emloft.net>,
	Ben Hutchings <bhutchings@...arflare.com>,
	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 Fri, 17 Aug 2012 11:55:43 +0400
Ilya Shchepetkov <shchepetkov@...ras.ru> wrote:

> >> Ben Hutchings <bhutchings@...arflare.com> writes:
> >>> 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.
> >> So you may want to add something like
> >>
> >> int register_netdev_carrier_off(struct net_device *dev)
> >> {
> >>      int err;
> >>
> >>      rtnl_lock();
> >>      err = register_netdevice(dev);
> >>         if (!err)
> >>                 set_bit(__LINK_STATE_NOCARRIER, &dev->state)
> >>      rtnl_unlock();
> >>      return err;
> >> }
> >>
> >>
> >> for these drivers?
> 
> t looks like this variant is equivalent to the existing code:
> 
> 	netif_carrier_off(dev);
> 	err = register_netdev(dev);
> 	if (err)
> 		goto out;
> 
> According to explanation in commit 22604c866889c4b2e12b73cbf1683bda1b72a313,
> in this case "this causes these drivers to incorrectly report their
> link status as IF_OPER_UNKNOWN which can falsely set the IFF_RUNNING
> flag when the interface is first brought up".
> 
> As far as I understand, to fix the issue it is required to call
> netif_carrier_off() itself:
> 
> int register_netdev_carrier_off(struct net_device *dev)
> {
> 	int err;
> 
> 	rtnl_lock();
> 	err = register_netdevice(dev);
> 	if (!err)
> 		netif_carrier_off(dev);
> 	rtnl_unlock();
> 	return err;
> }
> 
> What do you think?

Does this prevent  multiple link events from being reported to user space?

If the root cause of the problem is the link status
(commit 22604c866889c4b2e12b73cbf1683bda1b72a313), then the kernel
should be fixed to do link status correctly.

>From an application point of view IFF_RUNNING is meaningless unless IFF_UP
is set.
--
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