On Tue, 24 Feb 2004 viro@parcelfarce.linux.theplanet.co.uk wrote:
> I have an even better model for you:
>
> dev = alloc_netdev(<whatever>);
> netif_carrier_off(dev);
> free_netdev(dev);
>
> That leaves a scheduled work (by linkwatch_fire_event()) with pointer to
> freed dev.
>
> *NOTE*: dev_hold() provides protection only to devices that had been
> registered. So netif_carrier_off() is safe for those (dev_hold() will
> hold unregister_netdev() from from finishing and since the caller of
> unregister_netdev() is still holding a reference, device won't go away).
I'm impressed by your quick analysis. Thank you!
> IOW, Don't Do It, Then. netif_carrier_off() is not safe for devices
> that are not registered.
I won't do it for now. In the long term, I believe it's a legitimate
approach to register a network device with carrier off and set it to on
only if the carrier has been detected by the hardware.
Maybe linkwatch_fire_event() should do nothing in certain situations?
Following patch fixes the crash. I didn't put much thought into the
patch, perhaps it could be improved.
==================================
--- net/core/link_watch.c
+++ net/core/link_watch.c
@@ -102,6 +102,9 @@ static void linkwatch_event(void *dummy)
void linkwatch_fire_event(struct net_device *dev)
{
+ if (dev->reg_state == NETREG_UNINITIALIZED)
+ return;
+
if (!test_and_set_bit(__LINK_STATE_LINKWATCH_PENDING, &dev->state)) {
unsigned long flags;
struct lw_event *event;
==================================
--
Regards,
Pavel Roskin
-
: send the line "unsubscribe linux-net" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
