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  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:   Wed, 29 Apr 2020 12:38:58 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     snelson@...sando.io
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net 1/2] ionic: add LIF_READY state to close probe-open
 race

From: Shannon Nelson <snelson@...sando.io>
Date: Wed, 29 Apr 2020 11:37:38 -0700

> Add a bit of state to the lif to signify when the queues are
> ready to be used.  This closes an ionic_probe()/ionic_open()
> race condition where the driver has registered the netdev
> and signaled Link Up while running under ionic_probe(),
> which NetworkManager or other user processes can see and then
> try to bring up the device before the initial pass through
> ionic_link_status_check() has finished.  NetworkManager's
> thread can get into __dev_open() and set __LINK_STATE_START
> in dev->state before the ionic_probe() thread makes it to the
> netif_running() check, which results in the ionic_probe() thread
> trying to start the queues before the queues have completed
> their initialization.
> 
> Adding a LIF_QREADY flag allows us to prevent this condition by
> signaling whether the Tx/Rx queues are initialized and ready.
> 
> Fixes: c672412f6172 ("ionic: remove lifs on fw reset")
> Signed-off-by: Shannon Nelson <snelson@...sando.io>

I would rather you fix this by adjusting the visibility.

You should only call register_netdevice() when all of the device
setup and software state initialization is complete.

This improper ordering is the true cause of this bug and adding
this new boolean state is simply papering over the core issue.

Thanks.

Powered by blists - more mailing lists