netdev - Re: [RFC PATCH] net: Always fire at least one linkwatch event

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]

Date:	Tue, 27 Sep 2011 14:59:43 -0400 (EDT)
From:	David Miller <davem@...hat.com>
To:	nhorman@...driver.com
Cc:	netdev@...r.kernel.org, jfeeney@...hat.com
Subject: Re: [RFC PATCH] net: Always fire at least one linkwatch event

From: Neil Horman <nhorman@...driver.com>
Date: Wed, 21 Sep 2011 15:51:29 -0400

> It was recently noted that the tg3 driver had a problem in that after boot a
> kernel and if-upping the tg3 interface the sysfs operstate attribute continued
> to read 'unkown'.  This was happening because tg3 assumes the default carrier
> state (which is to say the __LINK_STATE_NOCARRIER bit is clear) is correct.
> That said, when the device is if-upped, and the open path, calls
> netif_carrier_on, the test_and_set_bit call in that function returns false
> (since the bit was previously zero from its initial state).  This means that
> netif_carrier_on call never generates a linkwatch event, and as a result
> dev->operstate never gets recomputed.  This could be fixed by unconditionally
> calling netif_carrier_off in the probe routine, to simply force a state change
> on that bit, but that seems like a sub-par solution, given that many drivers may
> have this error.  Instead it seems like it might be better to burn an extra bit
> in the state field to indicate that the CARRIER bit is still in the initial
> state and our first call to netif_carrier_[off|on] should always fire a
> linkwatch event.

I'm finding this analysis hard to follow.

tg3_open() does netif_carrier_off(), and this will set the
__LINK_STATE_NOCARRIER bit.

And since the registration state of the device is not
NETREG_UNINITIALIZED it will fire off a linkwatch event too.

So whenever the netif_carrier_on() happens later, the bit will be set
when the test_and_clear_bit() happens there.  So the
test_and_clear_bit() will not return false.

The registration state is not NETREG_UNINITIALIZED, so (again) that
will not block the linkwatch event from firing.
--
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