[Intel-wired-lan] [PATCH] ixgbevf: handle race between close and suspend on shutdown

Alexander Duyck alexander.duyck at gmail.com
Mon Oct 31 21:30:24 UTC 2016


On Mon, Oct 31, 2016 at 1:00 PM, Emil Tantilov <emil.s.tantilov at intel.com>
wrote:

> When an interface is part of a namespace it is possible that
> ixgbevf_close() may be called while ixgbevf_suspend() is running
> which ends up in a double free WARN and/or a BUG in free_msi_irqs()
>
> This patch introduces a new adapter->state bit that will allow us
> to handle this rate atomically. The idea is that ixgbevf_close() should
> not run if ixgbevf_suspend is in session.
>
> Signed-off-by: Emil Tantilov <emil.s.tantilov at intel.com>
>

I'm pretty sure this isn't the correct fix for this.  You still have the
two technically race against each other because the close could get caught
behind the rtnl lock and then flow through trying to free the interrupt
from it's side.  Also there are scenarios where this can leak interrupts
and such since suspend could set the bit, close could run through take the
rtnl lock, see the bit set so it doesn't free anything, then suspend
resumes and netif_running returns false so you never free the ring
resources.

I think the correct fix for this is to move the rtnl_lock/unlock so that
they are outside of the netif_device_detach() and
ixgbevf_clear_interrupt_scheme() calls that are a part of ixgbevf_suspend.
That way you shouldn't race with close.

It also looks like there is a similar bug in
the ixgbevf_queue_reset_subtask().  It should be taking the rtnl_lock
before checking netif_running, and releasing it at the end of the
function.  You could also add a call to netif_device_present to
ixgbevf_close and if it returns false then just return since the rtnl_lock
will guarantee that if it is set then suspend will have already freed the
resources.

- Alex
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20161031/ae8fc41e/attachment.html>


More information about the Intel-wired-lan mailing list