[PATCH net 0/3] ionic: fix port_info lifetime problems around device reset

[PATCH net 0/3] ionic: fix port_info lifetime problems around device reset

[Eric Joyner]

The ionic port_info DMA buffer is read by most of the driver's ethtool
ops, and is freed and reallocated by the firmware recovery and PCI reset
paths. Those paths hold no lock that the ethtool ops honor - the
driver takes no rtnl anywhere - so ethtool can dereference a pointer
that has just been set to NULL, or read out of a buffer that has just
been freed.

Patch 1 adds the missing NULL checks to the six ethtool ops that lacked
them; ionic_get_link_ext_stats() and ionic_get_link_ksettings() already
had them. It fixes the oops reachable today and does not depend on the
other two, so it stands on its own if the approach in patch 2 needs
more discussion.

Patch 2 detaches the netdev in ionic_reset_prepare(). The ethtool core
only stays out of a driver when netif_device_present() is false, and the
firmware recovery path already relies on that; the PCI reset path never
did. This is what actually closes the window rather than papering over
it.

Patch 3 fixes an unrelated leak of the same buffer that turned up while
auditing the free paths: probe failures after the port has been set up
unwind through a label that never calls ionic_port_reset().

The port_info lifetime problems were pointed out by the netdev AI review
bot on an unrelated ionic ethtool patch:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260731214021.15279-1-eric.joyner@amd.com

Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
Eric Joyner (3):
      ionic: check for a NULL port_info in the remaining ethtool ops
      ionic: detach the netdev in the PCI reset handler
      ionic: free port_info when probe fails after the port is set up

 .../net/ethernet/pensando/ionic/ionic_bus_pci.c    |  2 ++
 .../net/ethernet/pensando/ionic/ionic_ethtool.c    | 30 ++++++++++++++++++++++
 2 files changed, 32 insertions(+)
---
base-commit: 24ef02f934eeb48830cff6b739abc3c62b1d107b
change-id: 20260814-ionic-port-info-lifetime-3163779cb298

Best regards,
-- 
Eric Joyner <eric.joyner@amd.com>

[PATCH net 1/3] ionic: check for a NULL port_info in the remaining ethtool ops

[Eric Joyner]

port_info is a coherent DMA buffer that the firmware keeps up to date.
ionic_port_init() frees it and sets idev->port_info to NULL when the
device command to initialize the port fails, and that failure path can
run while the netdev is still registered:

  ionic_lif_deferred_work()
   -> ionic_lif_handle_fw_up()
       -> ionic_port_init()

  ionic_reset_done()
   -> ionic_setup_one()
       -> ionic_port_init()

ionic_get_link_ext_stats() and ionic_get_link_ksettings() already test
the pointer before using it, but the rest of the ethtool ops dereference
it blindly, so an unprivileged "ethtool --show-fec eth0" can oops after
a failed firmware recovery.

Add the same check to the ops that were missing it.

Fixes: c672412f6172 ("ionic: remove lifs on fw reset")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 .../net/ethernet/pensando/ionic/ionic_ethtool.c    | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index c4ab4b5caa0a..0830422fe7ba 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -347,6 +347,11 @@ static int ionic_set_link_ksettings(struct net_device *netdev,
 	if (test_bit(IONIC_LIF_F_FW_RESET, lif->state))
 		return -EBUSY;
 
+	if (!idev->port_info) {
+		netdev_err(netdev, "port_info not initialized\n");
+		return -EOPNOTSUPP;
+	}
+
 	/* set autoneg */
 	if (ks->base.autoneg != idev->port_info->config.an_enable) {
 		mutex_lock(&ionic->dev_cmd_lock);
@@ -378,6 +383,11 @@ static void ionic_get_pauseparam(struct net_device *netdev,
 
 	pause->autoneg = 0;
 
+	if (!lif->ionic->idev.port_info) {
+		netdev_err_once(netdev, "port_info not initialized\n");
+		return;
+	}
+
 	pause_type = lif->ionic->idev.port_info->config.pause_type;
 	if (pause_type) {
 		pause->rx_pause = (pause_type & IONIC_PAUSE_F_RX) ? 1 : 0;
@@ -396,6 +406,11 @@ static int ionic_set_pauseparam(struct net_device *netdev,
 	if (test_bit(IONIC_LIF_F_FW_RESET, lif->state))
 		return -EBUSY;
 
+	if (!lif->ionic->idev.port_info) {
+		netdev_err(netdev, "port_info not initialized\n");
+		return -EOPNOTSUPP;
+	}
+
 	if (pause->autoneg)
 		return -EOPNOTSUPP;
 
@@ -424,6 +439,11 @@ static int ionic_get_fecparam(struct net_device *netdev,
 {
 	struct ionic_lif *lif = netdev_priv(netdev);
 
+	if (!lif->ionic->idev.port_info) {
+		netdev_err(netdev, "port_info not initialized\n");
+		return -EOPNOTSUPP;
+	}
+
 	switch (lif->ionic->idev.port_info->config.fec_type) {
 	case IONIC_PORT_FEC_TYPE_NONE:
 		fec->active_fec = ETHTOOL_FEC_OFF;
@@ -451,6 +471,11 @@ static int ionic_set_fecparam(struct net_device *netdev,
 	if (test_bit(IONIC_LIF_F_FW_RESET, lif->state))
 		return -EBUSY;
 
+	if (!lif->ionic->idev.port_info) {
+		netdev_err(netdev, "port_info not initialized\n");
+		return -EOPNOTSUPP;
+	}
+
 	if (lif->ionic->idev.port_info->config.an_enable) {
 		netdev_err(netdev, "FEC request not allowed while autoneg is enabled\n");
 		return -EINVAL;
@@ -1004,6 +1029,11 @@ static int ionic_get_module_eeprom_by_page(struct net_device *netdev,
 		return -EINVAL;
 	}
 
+	if (!idev->port_info) {
+		NL_SET_ERR_MSG_MOD(extack, "port_info not initialized");
+		return -EOPNOTSUPP;
+	}
+
 	switch (page_data->page) {
 	case 0:
 		src = &idev->port_info->status.xcvr.sprom[page_data->offset];

-- 
2.43.0

[PATCH net 2/3] ionic: detach the netdev in the PCI reset handler

[Eric Joyner]

ionic_reset_prepare() tears down the queues, unmaps the BARs and calls
ionic_dev_teardown(), but leaves the netdev attached and registered for
the whole reset window. The ethtool core only refuses to call into a
driver when netif_device_present() is false, so ethtool ops keep running
against a device that is being dismantled underneath them, and
ionic_reset_done() can free idev->port_info out from under one of them
via ionic_setup_one() -> ionic_port_init().

The firmware recovery path already handles this properly:
ionic_lif_handle_fw_down() detaches before tearing anything down; so do
the same here.

No matching netif_device_attach() is needed on the way back up, since
ionic_reset_done() completes through ionic_restart_lif(), which already
re-attaches once the queues are alive again.

Fixes: a79b559e99be ("ionic: add FLR recovery support")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 05f19489ec5c..c15c4c705155 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -447,6 +447,7 @@ static void ionic_reset_prepare(struct pci_dev *pdev)
 	dev_dbg(ionic->dev, "%s: device stopping\n", __func__);
 
 	set_bit(IONIC_LIF_F_FW_RESET, lif->state);
+	netif_device_detach(lif->netdev);
 
 	timer_delete_sync(&ionic->watchdog_timer);
 	cancel_work_sync(&lif->deferred.work);

-- 
2.43.0

[PATCH net 3/3] ionic: free port_info when probe fails after the port is set up

[Eric Joyner]

ionic_probe() allocates the port_info DMA buffer via ionic_setup_one()
-> ionic_port_init(). If probe then fails anywhere after that, it
unwinds through err_out_pci, which never calls ionic_port_reset();
ionic_dev_teardown() and ionic_clear_pci() do not touch port_info, and
so the buffer is leaked.

Since ionic_remove() already frees the port_info DMA buffer with
ionic_port_reset(), call that on the probe error path too.
ionic_port_reset() returns early when port_info is NULL, so it is safe
for the earlier gotos that land on the same label before the port was
ever set up.

Fixes: 04436595c435 ("ionic: Add port management commands")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index c15c4c705155..2fb8795189b7 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -397,6 +397,7 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 err_out_free_irqs:
 	ionic_bus_free_irq_vectors(ionic);
 err_out_pci:
+	ionic_port_reset(ionic);
 	ionic_dev_teardown(ionic);
 	ionic_clear_pci(ionic);
 	ionic_debugfs_del_dev(ionic);

-- 
2.43.0

Re: [PATCH net 1/3] ionic: check for a NULL port_info in the remaining ethtool ops

[Vadim Fedorenko]

On 15/08/2026 01:00, Eric Joyner wrote:
> port_info is a coherent DMA buffer that the firmware keeps up to date.
> ionic_port_init() frees it and sets idev->port_info to NULL when the
> device command to initialize the port fails, and that failure path can
> run while the netdev is still registered:
> 
>    ionic_lif_deferred_work()
>     -> ionic_lif_handle_fw_up()
>         -> ionic_port_init()
> 
>    ionic_reset_done()
>     -> ionic_setup_one()
>         -> ionic_port_init()
> 
> ionic_get_link_ext_stats() and ionic_get_link_ksettings() already test
> the pointer before using it, but the rest of the ethtool ops dereference
> it blindly, so an unprivileged "ethtool --show-fec eth0" can oops after
> a failed firmware recovery.
> 
> Add the same check to the ops that were missing it.
> 
> Fixes: c672412f6172 ("ionic: remove lifs on fw reset")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Eric Joyner <eric.joyner@amd.com>
> ---
>   .../net/ethernet/pensando/ionic/ionic_ethtool.c    | 30 ++++++++++++++++++++++
>   1 file changed, 30 insertions(+)

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

Re: [PATCH net 2/3] ionic: detach the netdev in the PCI reset handler

[Vadim Fedorenko]

On 15/08/2026 01:00, Eric Joyner wrote:
> ionic_reset_prepare() tears down the queues, unmaps the BARs and calls
> ionic_dev_teardown(), but leaves the netdev attached and registered for
> the whole reset window. The ethtool core only refuses to call into a
> driver when netif_device_present() is false, so ethtool ops keep running
> against a device that is being dismantled underneath them, and
> ionic_reset_done() can free idev->port_info out from under one of them
> via ionic_setup_one() -> ionic_port_init().
> 
> The firmware recovery path already handles this properly:
> ionic_lif_handle_fw_down() detaches before tearing anything down; so do
> the same here.
> 
> No matching netif_device_attach() is needed on the way back up, since
> ionic_reset_done() completes through ionic_restart_lif(), which already
> re-attaches once the queues are alive again.
> 
> Fixes: a79b559e99be ("ionic: add FLR recovery support")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Eric Joyner <eric.joyner@amd.com>

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

Re: [PATCH net 3/3] ionic: free port_info when probe fails after the port is set up

[Vadim Fedorenko]

On 15/08/2026 01:00, Eric Joyner wrote:
> ionic_probe() allocates the port_info DMA buffer via ionic_setup_one()
> -> ionic_port_init(). If probe then fails anywhere after that, it
> unwinds through err_out_pci, which never calls ionic_port_reset();
> ionic_dev_teardown() and ionic_clear_pci() do not touch port_info, and
> so the buffer is leaked.
> 
> Since ionic_remove() already frees the port_info DMA buffer with
> ionic_port_reset(), call that on the probe error path too.
> ionic_port_reset() returns early when port_info is NULL, so it is safe
> for the earlier gotos that land on the same label before the port was
> ever set up.
> 
> Fixes: 04436595c435 ("ionic: Add port management commands")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Eric Joyner <eric.joyner@amd.com>
> ---
>   drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> index c15c4c705155..2fb8795189b7 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> @@ -397,6 +397,7 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   err_out_free_irqs:
>   	ionic_bus_free_irq_vectors(ionic);
>   err_out_pci:
> +	ionic_port_reset(ionic);
>   	ionic_dev_teardown(ionic);
>   	ionic_clear_pci(ionic);
>   	ionic_debugfs_del_dev(ionic);
> 
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

Re: [PATCH net 1/3] ionic: check for a NULL port_info in the remaining ethtool ops

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ionic: check for a NULL port_info in the remaining ethtool ops

This adds !idev->port_info checks to the ethtool ops that dereferenced the
coherent port_info DMA buffer without testing it, since ionic_port_init()
frees the buffer and stores NULL on device command failure while the netdev
is still registered.  The ops return -EOPNOTSUPP (or return early, for
ionic_get_pauseparam()) when the pointer is NULL.

A few questions below.

> ionic_get_link_ext_stats() and ionic_get_link_ksettings() already test
> the pointer before using it, but the rest of the ethtool ops dereference
> it blindly, so an unprivileged "ethtool --show-fec eth0" can oops after
> a failed firmware recovery.
>
> Add the same check to the ops that were missing it.

[Severity: High]
This isn't a bug introduced by this patch, but does the "rest of the
ethtool ops" also include .get_ethtool_stats?  That path still reaches an
unguarded dereference:

ionic_get_stats()            (ionic_ethtool.c)
 -> ionic_sw_stats_get_values()  (ionic_stats.c)
      port_stats = &lif->ionic->idev.port_info->stats;
      for (i = 0; i < IONIC_NUM_PORT_STATS; i++) {
              **buf = IONIC_READ_STAT_LE64(port_stats,
                                           &ionic_port_stats_desc[i]);

The only gate is in ionic_get_stats():

	if (test_bit(IONIC_LIF_F_FW_RESET, lif->state))
		return;

ionic_extra_port_stats_get_count() in the same file handles the same state
explicitly for the same ethtool -S request:

	if (!idev->port_info) {
		memset(pes_cache, 0xff, sizeof(*pes_cache));
		return count;
	}

so the two consumers of the same buffer disagree about whether the pointer
can be NULL.  Note also that ionic_remove() does
test_and_clear_bit(IONIC_LIF_F_FW_RESET, ...) before the netdev is
unregistered via ionic_lif_unregister(), so after a failed
ionic_reset_done() -> ionic_setup_one() -> ionic_port_init() the flag can be
clear, port_info NULL and the netdev still registered.  Should
ionic_sw_stats_get_values() get the same test the other ops just received?

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index c4ab4b5caa0ae..0830422fe7bae 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -347,6 +347,11 @@ static int ionic_set_link_ksettings(struct net_device *netdev,
>  	if (test_bit(IONIC_LIF_F_FW_RESET, lif->state))
>  		return -EBUSY;
>  
> +	if (!idev->port_info) {
> +		netdev_err(netdev, "port_info not initialized\n");
> +		return -EOPNOTSUPP;
> +	}
> +
>  	/* set autoneg */
>  	if (ks->base.autoneg != idev->port_info->config.an_enable) {
>  		mutex_lock(&ionic->dev_cmd_lock);

[Severity: High]
This isn't a bug introduced by this patch, since these ops previously
dereferenced port_info unconditionally, but can the added test still be
raced by the writer?  In ionic_port_init() the free and the NULL store
happen after the lock is dropped:

drivers/net/ethernet/pensando/ionic/ionic_main.c:ionic_port_init() {
	...
	mutex_unlock(&ionic->dev_cmd_lock);
	if (err) {
		dev_err(ionic->dev, "Failed to init port\n");
		dma_free_coherent(ionic->dev, idev->port_info_sz,
				  idev->port_info, idev->port_info_pa);
		idev->port_info = NULL;
	...
}

and its callers, ionic_lif_deferred_work() -> ionic_lif_handle_fw_up() and
ionic_reset_done() -> ionic_setup_one(), hold neither rtnl nor the netdev
instance lock that the ethtool ops run under.

In ionic_set_link_ksettings() the second dereference is separated from the
new test by a sleeping devcmd wait that releases the only lock the writer
takes:

	if (ks->base.autoneg != idev->port_info->config.an_enable) {
		mutex_lock(&ionic->dev_cmd_lock);
		ionic_dev_cmd_port_autoneg(idev, ks->base.autoneg);
		err = ionic_dev_cmd_wait(ionic, DEVCMD_TIMEOUT);
		mutex_unlock(&ionic->dev_cmd_lock);
	...
	/* set speed */
	if (ks->base.speed != le32_to_cpu(idev->port_info->config.speed)) {

Can the deferred fw-up work free the buffer and store NULL while the caller
sleeps in ionic_dev_cmd_wait(), so that the speed comparison dereferences
NULL?  ionic_set_fecparam() has the same shape with its two dereferences of
config.fec_type and config.an_enable, and ionic_get_module_eeprom_by_page()
computes src into the buffer after the test and then copies from it in
ionic_do_module_copy(), which would hand freed coherent memory to user
space.

> @@ -378,6 +383,11 @@ static void ionic_get_pauseparam(struct net_device *netdev,
>  
>  	pause->autoneg = 0;
>  
> +	if (!lif->ionic->idev.port_info) {
> +		netdev_err_once(netdev, "port_info not initialized\n");
> +		return;
> +	}
> +
>  	pause_type = lif->ionic->idev.port_info->config.pause_type;
>  	if (pause_type) {
>  		pause->rx_pause = (pause_type & IONIC_PAUSE_F_RX) ? 1 : 0;

[ ... ]

> @@ -424,6 +439,11 @@ static int ionic_get_fecparam(struct net_device *netdev,
>  {
>  	struct ionic_lif *lif = netdev_priv(netdev);
>  
> +	if (!lif->ionic->idev.port_info) {
> +		netdev_err(netdev, "port_info not initialized\n");
> +		return -EOPNOTSUPP;
> +	}
> +

[Severity: Low]
This isn't a bug, but is the difference in rate limiting between the two
getters intentional?  ionic_get_pauseparam() above uses netdev_err_once()
for this message, matching ionic_get_link_ext_stats(), while
ionic_get_fecparam() uses a plain netdev_err() for the identical message
and condition.  Both are unprivileged read-only ops, so a loop of
"ethtool --show-fec eth0" prints one KERN_ERR line per call while
port_info is NULL.

>  	switch (lif->ionic->idev.port_info->config.fec_type) {
>  	case IONIC_PORT_FEC_TYPE_NONE:
>  		fec->active_fec = ETHTOOL_FEC_OFF;
> @@ -451,6 +471,11 @@ static int ionic_set_fecparam(struct net_device *netdev,
>  	if (test_bit(IONIC_LIF_F_FW_RESET, lif->state))
>  		return -EBUSY;
>  
> +	if (!lif->ionic->idev.port_info) {
> +		netdev_err(netdev, "port_info not initialized\n");
> +		return -EOPNOTSUPP;
> +	}
> +
>  	if (lif->ionic->idev.port_info->config.an_enable) {
>  		netdev_err(netdev, "FEC request not allowed while autoneg is enabled\n");
>  		return -EINVAL;
> @@ -1004,6 +1029,11 @@ static int ionic_get_module_eeprom_by_page(struct net_device *netdev,
>  		return -EINVAL;
>  	}
>  
> +	if (!idev->port_info) {
> +		NL_SET_ERR_MSG_MOD(extack, "port_info not initialized");
> +		return -EOPNOTSUPP;
> +	}
> +
>  	switch (page_data->page) {
>  	case 0:
>  		src = &idev->port_info->status.xcvr.sprom[page_data->offset];

One more question about the failure path the commit message describes,
which the patch does not touch.

[Severity: Medium]
This is a pre-existing issue and not introduced by this patch, but is the
teardown in the ionic_port_init() error path complete?  The buffer address
is published to the device by ionic_dev_cmd_port_init():

	.port_init.info_pa = cpu_to_le64(idev->port_info_pa),

and on failure the buffer is released without an IONIC_CMD_PORT_RESET:

drivers/net/ethernet/pensando/ionic/ionic_main.c:ionic_port_init() {
	...
	ionic_dev_cmd_port_state(&ionic->idev, IONIC_PORT_ADMIN_STATE_UP);
	ionic_dev_cmd_wait(ionic, DEVCMD_TIMEOUT);

	mutex_unlock(&ionic->dev_cmd_lock);
	if (err) {
		dev_err(ionic->dev, "Failed to init port\n");
		dma_free_coherent(ionic->dev, idev->port_info_sz,
				  idev->port_info, idev->port_info_pa);
	...
}

ionic_port_reset() does the opposite ordering whenever the firmware is
still alive:

	if (ionic_is_fw_running(idev)) {
		mutex_lock(&ionic->dev_cmd_lock);
		ionic_dev_cmd_port_reset(idev);
		err = ionic_dev_cmd_wait(ionic, DEVCMD_TIMEOUT);
		mutex_unlock(&ionic->dev_cmd_lock);
	}

	dma_free_coherent(ionic->dev, idev->port_info_sz,
			  idev->port_info, idev->port_info_pa);

On the -ETIMEDOUT path __ionic_dev_cmd_wait() only calls
ionic_dev_cmd_clean() with no abort handshake, so can a slow-but-alive
firmware still write struct ionic_port_info into pages already returned to
the coherent allocator?  This depends on firmware behaviour, so it may well
be a non-issue.  Separately, the return value of the second devcmd
(PORT_STATE = IONIC_PORT_ADMIN_STATE_UP) is discarded just before the
free; is that deliberate?
-- 
pw-bot: cr

Re: [PATCH net 2/3] ionic: detach the netdev in the PCI reset handler

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ionic: detach the netdev in the PCI reset handler

Adds a netif_device_detach() call to ionic_reset_prepare() right after
IONIC_LIF_F_FW_RESET is set, so the netdev is marked not-present before the
PCI reset path tears down the queues, the BARs and the device state. The
change mirrors what ionic_lif_handle_fw_down() does on the firmware recovery
path, and relies on ionic_restart_lif() to re-attach afterwards.

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> index 05f19489ec5c8..c15c4c705155e 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> @@ -447,6 +447,7 @@ static void ionic_reset_prepare(struct pci_dev *pdev)
>  	dev_dbg(ionic->dev, "%s: device stopping\n", __func__);
>  
>  	set_bit(IONIC_LIF_F_FW_RESET, lif->state);
> +	netif_device_detach(lif->netdev);

[Severity: Medium]
Is the premise in the changelog accurate?  The message says
ionic_reset_prepare() "leaves the netdev attached and registered for the
whole reset window", but ionic_reset_prepare() already reaches
ionic_stop_queues_reconfig(), whose first statement is a detach:

drivers/net/ethernet/pensando/ionic/ionic_lif.c:
void ionic_stop_queues_reconfig(struct ionic_lif *lif)
{
	/* Stop and clean the queues before reconfiguration */
	netif_device_detach(lif->netdev);
	ionic_stop_queues(lif);
	ionic_txrx_deinit(lif);
}

That call runs before every destructive step in ionic_reset_prepare()
(ionic_txrx_free(), ionic_lif_deinit(), ionic_qcqs_free(),
ionic_debugfs_del_lif(), ionic_dev_teardown(), ionic_clear_pci()), and well
before ionic_reset_done() -> ionic_setup_one() -> ionic_port_init() runs.

netif_device_detach() clears __LINK_STATE_PRESENT unconditionally:

net/core/dev.c:
	if (test_and_clear_bit(__LINK_STATE_PRESENT, &dev->state) &&
	    netif_running(dev)) {
		netif_tx_stop_all_queues(dev);
	}

so the presence gate was already closed before the teardown even for an
interface that is not up.

The only window the new call additionally covers is:

	set_bit(IONIC_LIF_F_FW_RESET, lif->state);
	netif_device_detach(lif->netdev);
	timer_delete_sync(&ionic->watchdog_timer);
	cancel_work_sync(&lif->deferred.work);
	ionic_auxbus_unregister(ionic->lif);
	mutex_lock(&lif->queue_lock);

None of those steps free idev->port_info or unmap the BARs, so the
ionic_port_init() free named in the message does not appear to be the
exposure that moves here.  Would it be more accurate to describe this as
making ionic_reset_prepare() consistent with
ionic_lif_handle_fw_down() (detach up front, before the auxbus unregister
and the queue_lock section), and to reconsider the
Fixes: a79b559e99be tag so stable does not pick this up as the fix for the
described use-after-free?

[Severity: High]
This isn't a bug introduced by this patch, but since the changelog presents
the patch as closing the port_info use-after-free, is the added detach
sufficient to close it?

The ethtool core checks presence once, before dispatching to the driver:

net/ethtool/ioctl.c:dev_ethtool_locked() {
	if (!netif_device_present(dev)) {
		rc = -ENODEV;
		goto out;
	}
	...
}

ethnl_ops_begin() does the same on the netlink path.  netif_device_detach()
is only a test_and_clear_bit() plus netif_tx_stop_all_queues(), with no wait
or drain, so a caller already past that check keeps running.

ionic_reset_prepare() is called from the PCI core holding only device_lock;
it takes neither rtnl_lock() nor the netdev instance/ops lock that the
ethtool callbacks run under.  Can the following interleaving still happen?

	ethtool GLINKSETTINGS			PCI reset
	dev_ethtool_locked()
	  netif_device_present() -> true
	  ionic_get_link_ksettings()
	    reads idev->port_info		ionic_reset_prepare()
						  netif_device_detach()
						  ionic_clear_pci()
						  ionic_dev_teardown()
						ionic_reset_done()
						  ionic_setup_one()
						    ionic_port_init()
	    (still dereferencing)		      dma_free_coherent(...
						        idev->port_info)

ionic_clear_pci() also drops the register pointers an in-flight op may be
using:

drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c:ionic_clear_pci() {
	if (ionic->num_bars) {
		ionic->idev.dev_info_regs = NULL;
		ionic->idev.dev_cmd_regs = NULL;
		ionic->idev.intr_status = NULL;
		ionic->idev.intr_ctrl = NULL;

		ionic_unmap_bars(ionic);
		pci_release_regions(ionic->pdev);
	}
	...
}

and ionic_dev_teardown() frees idev->cmb_inuse, destroys ionic->wq and
mutex_destroy()s idev->cmb_inuse_lock while the netdev is still registered.

The NULL test on idev->port_info in ionic_get_link_ksettings(),
ionic_get_pauseparam() and ionic_get_fecparam(), and the
IONIC_LIF_F_FW_RESET test in the set-ops, are check-then-use against a
teardown path that holds no lock in common with them, so the new detach adds
a second advisory gate with the same property.

Would serializing the detach and the whole teardown/re-setup under the lock
the ethtool and ndo callbacks already hold (rtnl_lock() /
netdev_lock_ops()), or protecting idev->port_info with a lock taken on both
sides, be needed to actually close this?

>  
>  	timer_delete_sync(&ionic->watchdog_timer);
>  	cancel_work_sync(&lif->deferred.work);