[PATCH net v3] ice: fix Tx hang dump using stale tx_buf[0].skb

[PATCH net v3] ice: fix Tx hang dump using stale tx_buf[0].skb

[Vincent Choiniere via B4 Relay]

From: Vincent Choiniere <choiniere.vincent@hydroquebec.com>

ice_tx_hang_reporter_dump() always reads tx_ring->tx_buf[0].skb,
regardless of next_to_clean. The Tx cleanup path frees the skb and
marks the buffer empty without clearing the skb pointer, so slot 0 can
hold a stale pointer. A NULL check alone does not cover this, and
devlink_fmsg_dump_skb() can fault on the freed skb.

Using tx_buf[next_to_clean] alone does not fix the lifetime issues.
The dump runs from the service task without synchronization with Tx
cleanup or ring replacement:

- ice_clean_tx_irq() can free the skb at next_to_clean while the
  report is being prepared. It calls napi_consume_skb() before
  marking the buffer empty.
- The ring saved by ice_tx_timeout() can be replaced or freed before
  the service task runs. ethtool -G replaces the ring contents and
  frees tx_buf and desc, while ethtool -L can free whole rings.

Record the queue index, next_to_clean and next_to_use at timeout time.
Before reporting, take RTNL and check that the saved ring is still
one of the PF VSI Tx rings and that the VSI is up. Copy the
descriptors and IRQ name while the ring storage is protected against
replacement. If the ring cannot be used or the copy cannot be made,
report only the values recorded at timeout time.

Tx cleanup can still run while the ring state is copied, so do not
dereference the skb. Retain only its pointer value in the dump and
remove the skb contents.

Release RTNL before calling devlink_health_report(), which takes the
devlink instance lock. Build the dump from the copied data and free
the descriptor copy after reporting.

Observed on an E810 PF passed through to a KVM guest with Ubuntu
kernel 7.0.0-14 (PREEMPT_RT and a local scheduler patch): a delayed
queue IRQ led to a Tx timeout and a page fault in
devlink_fmsg_dump_skb():

 BUG: unable to handle page fault for address: 0000000004cd0c11
 Workqueue: ice ice_service_task [ice]
 RIP: devlink_fmsg_dump_skb+0x2d8/0x5d0
 Call Trace:
  ice_tx_hang_reporter_dump+0x1ce/0x210 [ice]
  devlink_health_do_dump+0xcc/0x190
  devlink_health_report+0x1d7/0x260
  ice_report_tx_hang+0x7d/0xa0 [ice]

Fixes: 2a82874a3b7b ("ice: add Tx hang devlink health reporter")
Link: https://lore.kernel.org/all/20260327072332.130320-1-aleksandr.loktionov@intel.com/
Cc: stable@vger.kernel.org
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Vincent Choiniere <choiniere.vincent@hydroquebec.com>
---
Changes in v3:
- Glue the kvfree() of the descriptor copy to the preceding if as an
  else (Przemek).
- Reword the ice_tx_hang_snapshot() kernel-doc: it said the rings are
  only replaced or freed under RTNL or from the service task, which does
  not hold for driver removal and devlink reload (Sashiko). No code
  change for this.
- Carry Przemek's Reviewed-by; the only code change is the else above.
- Move the AI-assistance attribution from the commit trailers to
  the notes below.

AI assistance: Claude (claude-sonnet-5-5).

Changes in v2:
- Validate the saved ring under RTNL before dereferencing it from
  the service task, and restrict snapshots to the PF VSI while it
  is up (Sashiko).
- Check next_to_clean against count before indexing tx_buf, and
  copy the descriptors and IRQ name while ring replacement is
  excluded (Sashiko).
- Stop dereferencing the skb and remove its contents from the dump.
  Keep only its pointer value (Sashiko).
- Release RTNL before reporting to preserve the devlink instance
  lock ordering.
- Record the queue index and ring indices at timeout time as a
  fallback when a snapshot cannot be made.
- Drop Paul's Reviewed-by because the patch changed substantially.

v2:
https://lore.kernel.org/netdev/20261026-ice-tx-hang-dump-v2-1-e2136ef2c5dc@hydroquebec.com/

v1:
https://lore.kernel.org/netdev/20261016-ice-tx-hang-dump-v1-1-3df0f5979a51@hydroquebec.com/

Testing:
- Built ice.ko with x86_64 defconfig, CONFIG_ICE=m, GCC 12.2 and W=1.
  Ran sparse with C=2 (sparse git c464045). Neither reported warnings
  in the changed files.
- checkpatch --strict reported only an unknown Fixes commit because
  the local tree is a shallow clone.
- Runtime testing used an E810 PF passed through to a KVM guest with
  Ubuntu kernel 7.0.0-38 (PREEMPT_RT), KASAN, PROVE_LOCKING and
  DEBUG_ATOMIC_SLEEP.
- Forced Tx timeouts by starving one queue's IRQ thread. The watchdog
  fired after five seconds, the PF recovered, and the dump retained
  the v1 fields except for the skb contents.
- Used a test-only kernel patch to insert configurable delays before
  ice_report_tx_hang() or the dump callback. Racing ethtool commands
  were started when the instrumentation logged the delay.
- With v1 and the same instrumentation, shrinking the ring with
  ethtool -G tx 64 while next_to_clean was above 64 triggered a KASAN
  slab-out-of-bounds report. This was also reproduced on a non-RT
  kernel built from net. Racing ethtool -L combined 1 triggered a
  use-after-free followed by a general protection fault.
- With v2, ran 168 forced Tx timeouts, including 149 with concurrent
  ethtool -G (tx 64, 128 or 512) or ethtool -L combined 1, before and
  after the ring state was copied. No KASAN, lockdep or
  sleeping-in-atomic reports were observed. When the saved ring was
  no longer present, the dump contained only the timeout-time values.

Limitations:
- The same stress testing was not completed on a non-RT kernel,
  where Tx timeouts could not be forced reliably.
- Subfunction VSIs and XDP were not tested. Subfunction VSI rings
  are excluded from the snapshot because ice_sf_dev_remove() frees
  them without RTNL.
- Driver removal and devlink reload still stop the service task only after
  the devlink reporter and the VSIs are torn down, and the service task
  runs the Tx hang report before it checks ICE_SERVICE_DIS. Before this
  patch the same path dereferenced the saved ring with no protection at
  all. The ordering is left to a separate fix.
- This patch does not synchronize publication and consumption of
  tx_hang_buf between ice_tx_timeout() and the service task. A newer
  timeout may still be dropped or its fields mixed with an older
  event.

The original devlink_fmsg_dump_skb() crash was reproduced on Ubuntu
kernel 7.0.0-14 with PREEMPT_RT and a local scheduler patch. The
additional race reproductions above used test-only instrumentation.
The affected reporter code is also present in upstream net at
191aa4154.
---
 drivers/net/ethernet/intel/ice/devlink/health.c | 118 ++++++++++++++++++++----
 drivers/net/ethernet/intel/ice/devlink/health.h |   6 ++
 2 files changed, 108 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/devlink/health.c b/drivers/net/ethernet/intel/ice/devlink/health.c
index 8e9a8a817..faad9271d 100644
--- a/drivers/net/ethernet/intel/ice/devlink/health.c
+++ b/drivers/net/ethernet/intel/ice/devlink/health.c
@@ -382,7 +382,13 @@ struct ice_tx_hang_event {
 	u16 queue;
 	u16 next_to_clean;
 	u16 next_to_use;
-	struct ice_tx_ring *tx_ring;
+	/* the fields below are only valid if @desc is set */
+	char irq_name[ICE_INT_NAME_STR_LEN];
+	void *desc_ptr;
+	dma_addr_t dma;
+	void *desc;
+	u32 desc_size;
+	void *skb_ptr;	/* pointer value only, never dereferenced */
 };
 
 static int ice_tx_hang_reporter_dump(struct devlink_health_reporter *reporter,
@@ -390,12 +396,10 @@ static int ice_tx_hang_reporter_dump(struct devlink_health_reporter *reporter,
 				     struct netlink_ext_ack *extack)
 {
 	struct ice_tx_hang_event *event = priv_ctx;
-	struct sk_buff *skb;
 
 	if (!event)
 		return 0;
 
-	skb = event->tx_ring->tx_buf->skb;
 	devlink_fmsg_obj_nest_start(fmsg);
 	ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, head);
 	ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, intr);
@@ -403,13 +407,14 @@ static int ice_tx_hang_reporter_dump(struct devlink_health_reporter *reporter,
 	ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, queue);
 	ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, next_to_clean);
 	ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, next_to_use);
-	devlink_fmsg_put(fmsg, "irq-mapping", event->tx_ring->q_vector->name);
-	ice_fmsg_put_ptr(fmsg, "desc-ptr", event->tx_ring->desc);
-	ice_fmsg_put_ptr(fmsg, "dma-ptr", (void *)(long)event->tx_ring->dma);
-	ice_fmsg_put_ptr(fmsg, "skb-ptr", skb);
-	devlink_fmsg_binary_pair_put(fmsg, "desc", event->tx_ring->desc,
-				     event->tx_ring->count * sizeof(struct ice_tx_desc));
-	devlink_fmsg_dump_skb(fmsg, skb);
+	if (event->desc) {
+		devlink_fmsg_put(fmsg, "irq-mapping", event->irq_name);
+		ice_fmsg_put_ptr(fmsg, "desc-ptr", event->desc_ptr);
+		ice_fmsg_put_ptr(fmsg, "dma-ptr", (void *)(long)event->dma);
+		ice_fmsg_put_ptr(fmsg, "skb-ptr", event->skb_ptr);
+		devlink_fmsg_binary_pair_put(fmsg, "desc", event->desc,
+					     event->desc_size);
+	}
 	devlink_fmsg_obj_nest_end(fmsg);
 
 	return 0;
@@ -424,24 +429,105 @@ void ice_prep_tx_hang_report(struct ice_pf *pf, struct ice_tx_ring *tx_ring,
 	buf->vsi_num = vsi_num;
 	buf->head = head;
 	buf->intr = intr;
+	buf->queue = tx_ring->q_index;
+	buf->next_to_clean = tx_ring->next_to_clean;
+	buf->next_to_use = tx_ring->next_to_use;
+}
+
+/**
+ * ice_tx_hang_snapshot - copy the hung Tx ring state for the dump
+ * @pf: PF on which the Tx hang was detected
+ * @tx_ring: ring saved by ice_tx_timeout(), possibly stale
+ * @ev: event to fill in
+ *
+ * The saved ring may have been freed or replaced since the timeout (e.g. by
+ * ethtool -G or -L). Use it only if it is still one of the PF VSI Tx rings
+ * and the VSI is up. Those paths, and the reset paths, replace or free the
+ * rings under RTNL or from the service task itself, so with RTNL held the
+ * accesses below are to live memory for them. Driver removal and devlink
+ * reload tear the VSIs down before the service task is stopped and are not
+ * covered here. ice_clean_tx_irq() may still run and free the skb at
+ * next_to_clean at any time, so only the pointer value is recorded and the
+ * skb itself is never dereferenced.
+ */
+static void ice_tx_hang_snapshot(struct ice_pf *pf, struct ice_tx_ring *tx_ring,
+				 struct ice_tx_hang_event *ev)
+{
+	struct ice_vsi *vsi = ice_get_main_vsi(pf);
+	struct ice_q_vector *q_vector;
+	struct ice_tx_buf *tx_buf;
+	unsigned int i;
+	u16 ntc, ntu;
+	void *desc;
+	u32 size;
+
+	ASSERT_RTNL();
+
+	if (!tx_ring || !vsi || !vsi->tx_rings ||
+	    test_bit(ICE_VSI_DOWN, vsi->state))
+		return;
+
+	ice_for_each_txq(vsi, i)
+		if (vsi->tx_rings[i] == tx_ring)
+			break;
+	if (i == vsi->num_txq)
+		return;
+
+	q_vector = tx_ring->q_vector;
+	if (!q_vector || !tx_ring->desc || !tx_ring->tx_buf || !tx_ring->count)
+		return;
+
+	size = tx_ring->count * sizeof(struct ice_tx_desc);
+	desc = kvmalloc(size, GFP_KERNEL);
+	if (!desc)
+		return;
+
+	ntc = READ_ONCE(tx_ring->next_to_clean);
+	ntu = READ_ONCE(tx_ring->next_to_use);
+
+	if (ntc < tx_ring->count) {
+		tx_buf = &tx_ring->tx_buf[ntc];
+		if (tx_buf->type == ICE_TX_BUF_SKB)
+			ev->skb_ptr = READ_ONCE(tx_buf->skb);
+
+		memcpy(desc, tx_ring->desc, size);
+		ev->next_to_clean = ntc;
+		ev->next_to_use = ntu;
+		strscpy(ev->irq_name, q_vector->name);
+		ev->desc_ptr = tx_ring->desc;
+		ev->dma = tx_ring->dma;
+		ev->desc_size = size;
+		ev->desc = desc;
+	} else {
+		kvfree(desc);
+	}
 }
 
 void ice_report_tx_hang(struct ice_pf *pf)
 {
 	struct ice_health_tx_hang_buf *buf = &pf->health_reporters.tx_hang_buf;
-	struct ice_tx_ring *tx_ring = buf->tx_ring;
-
 	struct ice_tx_hang_event ev = {
 		.head = buf->head,
 		.intr = buf->intr,
 		.vsi_num = buf->vsi_num,
-		.queue = tx_ring->q_index,
-		.next_to_clean = tx_ring->next_to_clean,
-		.next_to_use = tx_ring->next_to_use,
-		.tx_ring = tx_ring,
+		.queue = buf->queue,
+		.next_to_clean = buf->next_to_clean,
+		.next_to_use = buf->next_to_use,
 	};
 
+	if (!pf->health_reporters.tx_hang)
+		return;
+
+	rtnl_lock();
+	ice_tx_hang_snapshot(pf, buf->tx_ring, &ev);
+	rtnl_unlock();
+
+	/* devlink_health_report() takes the devlink instance lock, which nests
+	 * outside RTNL, so only report once RTNL has been released.
+	 */
 	ice_devlink_health_report(pf->health_reporters.tx_hang, "Tx hang", &ev);
+
+	kvfree(ev.desc);
 }
 
 static struct devlink_health_reporter *
diff --git a/drivers/net/ethernet/intel/ice/devlink/health.h b/drivers/net/ethernet/intel/ice/devlink/health.h
index 5edfc4d2a..508e4115d 100644
--- a/drivers/net/ethernet/intel/ice/devlink/health.h
+++ b/drivers/net/ethernet/intel/ice/devlink/health.h
@@ -37,6 +37,9 @@ enum ice_mdd_src {
  * @head: descriptor head
  * @intr: interrupt register value
  * @vsi_num: VSI owning the queue that the hang occurred on
+ * @queue: index of the queue that the hang occurred on
+ * @next_to_clean: next_to_clean of the ring when the hang was detected
+ * @next_to_use: next_to_use of the ring when the hang was detected
  * @fw_status: buffer for last received FW Status event
  * @port_status: buffer for last received Port Status event
  */
@@ -50,6 +53,9 @@ struct ice_health {
 		u32 head;
 		u32 intr;
 		u16 vsi_num;
+		u16 queue;
+		u16 next_to_clean;
+		u16 next_to_use;
 	);
 	struct ice_aqc_health_status_elem fw_status;
 	struct ice_aqc_health_status_elem port_status;

---
base-commit: 7076f684d5132e6a6145e1da2fa74771ecd052e5
change-id: 20261016-ice-tx-hang-dump-e46b4ed483d6

Best regards,
--  
Vincent Choiniere <choiniere.vincent@hydroquebec.com>

Re: [PATCH net v3] ice: fix Tx hang dump using stale tx_buf[0].skb

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 (2) · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] ice_tx_hang_snapshot()
  (drivers/net/ethernet/intel/ice/devlink/health.c) decides whether the…
- [Low] This patch removes the only caller of devlink_fmsg_dump_skb(),
  which was ice_tx_hang_reporter_dump().
- [Low] The new fields tx_hang_buf.queue, next_to_clean and next_to_use
  use the existing unsynchronized handoff from ice_tx_timeout() to the…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the service
  task can still run ice_report_tx_hang() during driver removal after…
- [High] This problem wasn't introduced by this patch, but devlink reload
  with DRIVER_REINIT can deadlock against the service task's Tx-hang…

--

ice: fix Tx hang dump using stale tx_buf[0].skb

This patch stops ice_tx_hang_reporter_dump() from dereferencing the saved
Tx ring and a skb that may already be freed. It records the queue index
and ring indices at timeout time. It checks the saved ring under RTNL
before copying the descriptors and IRQ name, and it removes the skb
contents from the dump.

Link: https://lore.kernel.org/all/20260327072332.130320-1-aleksandr.loktionov@intel.com/

> diff --git a/drivers/net/ethernet/intel/ice/devlink/health.c b/drivers/net/ethernet/intel/ice/devlink/health.c
> index 8e9a8a8178d48..faad9271df0f6 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/health.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/health.c

[ ... ]

> @@ -403,13 +407,14 @@ static int ice_tx_hang_reporter_dump(struct devlink_health_reporter *reporter,
>  	ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, queue);
>  	ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, next_to_clean);
>  	ICE_DEVLINK_FMSG_PUT_FIELD(fmsg, event, next_to_use);
> -	devlink_fmsg_put(fmsg, "irq-mapping", event->tx_ring->q_vector->name);
> -	ice_fmsg_put_ptr(fmsg, "desc-ptr", event->tx_ring->desc);
> -	ice_fmsg_put_ptr(fmsg, "dma-ptr", (void *)(long)event->tx_ring->dma);
> -	ice_fmsg_put_ptr(fmsg, "skb-ptr", skb);
> -	devlink_fmsg_binary_pair_put(fmsg, "desc", event->tx_ring->desc,
> -				     event->tx_ring->count * sizeof(struct ice_tx_desc));
> -	devlink_fmsg_dump_skb(fmsg, skb);

[Severity: Low]
This isn't a bug, but this removes the only caller of
devlink_fmsg_dump_skb(). What remains is its definition and
EXPORT_SYMBOL_GPL in net/devlink/health.c and its prototype in
include/net/devlink.h, with no users.

Could the commit message say whether removing the unused export is
planned as a net-next follow-up?

[ ... ]

> @@ -424,24 +429,105 @@ void ice_prep_tx_hang_report(struct ice_pf *pf, struct ice_tx_ring *tx_ring,
>  	buf->vsi_num = vsi_num;
>  	buf->head = head;
>  	buf->intr = intr;
> +	buf->queue = tx_ring->q_index;
> +	buf->next_to_clean = tx_ring->next_to_clean;
> +	buf->next_to_use = tx_ring->next_to_use;
> +}

[Severity: Low]
These new fields are stored after buf->tx_ring, which ice_service_task()
uses as the pending-event flag. The stores are plain, with no release
ordering:

ice_prep_tx_hang_report() {
	buf->tx_ring = tx_ring;
	...
	buf->queue = tx_ring->q_index;
	...
}

ice_service_task() {
	if (pf->health_reporters.tx_hang_buf.tx_ring) {
		ice_report_tx_hang(pf);
		pf->health_reporters.tx_hang_buf.tx_ring = NULL;
	}
}

ice_report_tx_hang() copies buf->queue, head, intr and the indices into
ev before rtnl_lock(). It then reads buf->tx_ring again under RTNL and
passes that to ice_tx_hang_snapshot().

A newer ice_tx_timeout() could overwrite buf while the service task waits
on RTNL. Could the dump then show one ring's descriptors, IRQ name and
indices next to another event's queue, head and intr? The newer event
would also be dropped when ice_service_task() clears tx_ring.

Before this patch, queue and the indices came from the same tx_ring
pointer that was dumped, so they always matched. The patch notes
acknowledge that the handoff is unsynchronized. Could the success path of
ice_tx_hang_snapshot() at least set ev->queue = tx_ring->q_index, the
same way it updates next_to_clean and next_to_use?

[ ... ]

> +static void ice_tx_hang_snapshot(struct ice_pf *pf, struct ice_tx_ring *tx_ring,
> +				 struct ice_tx_hang_event *ev)
> +{

[ ... ]

> +	ice_for_each_txq(vsi, i)
> +		if (vsi->tx_rings[i] == tx_ring)
> +			break;
> +	if (i == vsi->num_txq)
> +		return;

[Severity: Medium]
Is pointer equality enough to show that this is still the hung ring? Two
cases seem to pass this check.

With ethtool -G, the ring contents are replaced in place. The struct keeps
its address, but desc, tx_buf, count and the indices are new:

ice_set_ringparam() {
	...
	ice_free_tx_ring(vsi->tx_rings[i]);
	*vsi->tx_rings[i] = tx_rings[i];
	...
}

With ethtool -L, ice_vsi_clear_rings() frees the old rings with
kfree_rcu(), and ice_vsi_alloc_rings() allocates new ones with
ring->q_index = i. A new ring for a different queue could end up at the
freed address.

In both cases the snapshot succeeds. It overwrites the timeout-time
next_to_clean and next_to_use with the live values of a different ring.
It also copies that ring's descriptors, IRQ name, desc pointer and DMA
address. Meanwhile queue, head and intr still describe the original hung
queue.

The commit message says:

    If the ring cannot be used or the copy cannot be made,
    report only the values recorded at timeout time.

Neither case is detected here, though. Would checking i == ev->queue (or
tx_ring->q_index == ev->queue) at least reject the cross-queue case?

[ ... ]

>  void ice_report_tx_hang(struct ice_pf *pf)
>  {

[ ... ]

> +	if (!pf->health_reporters.tx_hang)
> +		return;
> +
> +	rtnl_lock();
> +	ice_tx_hang_snapshot(pf, buf->tx_ring, &ev);
> +	rtnl_unlock();

[Severity: High]
This isn't a bug introduced by this patch, but can the service task reach
this code during driver removal, after the tx_hang reporter has been
freed?

ice_remove() destroys the reporter and releases the VSIs before it stops
the service task:

ice_remove()
    ice_deinit_devlink(pf)
        ice_health_deinit()
            ice_deinit_devl_reporter()
                devl_health_reporter_destroy()   <- reporter freed
    ice_unload(pf)
    ice_deinit(pf)
    ice_vsi_release_all(pf)                      <- no RTNL held
    ...
    ice_deinit_dev(pf)
        ice_service_task_stop()
            cancel_work_sync(&pf->serv_task)

pf->health_reporters.tx_hang is not cleared after the destroy, so the new
NULL check sees a dangling pointer. ice_service_task() calls
ice_report_tx_hang() before checking ICE_SERVICE_DIS, whenever
tx_hang_buf.tx_ring is set. A Tx timeout before ice_unload() is enough to
set it.

devlink_health_report() would then update error_count and health_state
and call devlink_health_do_dump() on the freed reporter. The same applies
to a service task that is already blocked in devl_lock() inside
devlink_health_report() while ice_remove() destroys the reporter. It
reaches the freed reporter once it gets the lock.

ice_tx_hang_snapshot() also dereferences pf->vsi[0], vsi->tx_rings,
vsi->state and tx_ring->desc under RTNL. ice_vsi_release_all() runs
without RTNL in ice_remove(), so holding RTNL does not keep those objects
alive.

The patch notes list this ordering as a known limitation to be fixed
separately.

> +
> +	/* devlink_health_report() takes the devlink instance lock, which nests
> +	 * outside RTNL, so only report once RTNL has been released.
> +	 */
>  	ice_devlink_health_report(pf->health_reporters.tx_hang, "Tx hang", &ev);

[Severity: High]
This is a pre-existing issue, but can a devlink reload with DRIVER_REINIT
deadlock against this report?

The devlink core holds devl_lock while it runs the reload down path, and
that path waits for the service task:

devlink reload (devl_lock held)
    ice_devlink_reload_down()
        ice_devlink_reinit_down()
            ice_deinit_dev()
                ice_service_task_stop()
                    cancel_work_sync(&pf->serv_task)

If a Tx timeout set tx_hang_buf.tx_ring just before the reload, the
service task runs:

ice_service_task()
    ice_report_tx_hang()
        devlink_health_report()
            devl_lock(devlink)    <- auto_dump path, blocks

auto_dump defaults to true for this reporter because it has a .dump op.
The service task waits for devl_lock, which the reload thread holds. The
reload thread waits in cancel_work_sync() for the service task.

Releasing RTNL first keeps RTNL out of the cycle, but the devl_lock
dependency against reload is still there.

The FW_ACTIVATE reload path seems to have a milder version of this.
ice_devlink_reload_empr_finish() waits for the reset under devl_lock.
The blocked service task cannot run ice_reset_subtask(), so the wait
times out.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261107-ice-tx-hang-dump-v3-1-8f22109d1aad%40hydroquebec.com