[PATCH iwl-net] e1000e: close and reattach the netdev when e1000e_pm_thaw() fails

[PATCH iwl-net] e1000e: close and reattach the netdev when e1000e_pm_thaw() fails

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

e1000e_pm_thaw() re-requests the interrupts that e1000e_pm_freeze()
released and jumps to the err_irq label when e1000_request_irq()
fails, which skips netif_device_attach(). The netdev is therefore left
detached while the interface is still IFF_UP: e1000e_down() stopped
the transmit queue and no interrupt is registered, but __dev_open()
refuses to open a netdev that is not present, so the interface can be
brought down and never up again. e1000e_pm_freeze() keeps it in that
state as well, since it only tears the interface down when the device
was present.

Attaching the netdev unconditionally is not enough. The driver uses
netif_device_present() as the marker for "the interrupts are
registered" - both e1000e_pm_freeze() and e1000e_close() rely on it to
decide whether e1000_free_irq() has to be called - so attaching an
interface whose e1000_request_irq() failed would make the next
teardown call free_irq() on a line that has no handler of ours
registered and trip the "Trying to free already-free IRQ" warning.

Close the interface while the netdev is still detached instead, so
that e1000e_close() skips the hardware and interrupt teardown, and
attach the netdev right afterwards. The interface is then in a
consistent state: the rings are released, NAPI is disabled, no
interrupt is registered and user space can bring it up again, which
requests the interrupts from e1000e_open().

Fixes: 2800209994f8 ("e1000e: Refactor PM flows")
Cc: stable@vger.kernel.org
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 7ae33273734f..6da613025a1a 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6911,7 +6911,19 @@ static int e1000e_pm_thaw(struct device *dev)
 	}
 
 	netif_device_attach(netdev);
+	rtnl_unlock();
+
+	return rc;
+
 err_irq:
+	/* The interrupts could not be restored, so the interface cannot
+	 * be used. Close it while the netdev is still detached, which
+	 * makes e1000e_close() skip the hardware and interrupt teardown,
+	 * and attach the netdev afterwards so that it can be brought up
+	 * again.
+	 */
+	dev_close(netdev);
+	netif_device_attach(netdev);
 	rtnl_unlock();
 
 	return rc;
-- 
2.25.1

Re: [PATCH iwl-net] e1000e: close and reattach the netdev when e1000e_pm_thaw() fails

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.