All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Daniel Machon" <daniel.machon@microchip.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH net-next v2 08/13] net: lan966x: add shutdown callback to stop FDMA on reboot
Date: Wed, 29 Apr 2026 13:10:04 +0000	[thread overview]
Message-ID: <20260429131004.A671AC2BCB8@smtp.kernel.org> (raw)
In-Reply-To: <20260428-lan966x-pci-fdma-v2-8-d3ec66e06202@microchip.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `lan966x_shutdown()` callback fails to detach the network stack and NAPI, allowing concurrent network activity to completely undo the shutdown sequence.
- [High] Use-After-Free in `lan966x_remove()` if packets are transmitted during driver teardown.
--

Note: The format of this review has been altered and inline code quoting
omitted due to recitation restrictions.

commit a6cbd3c8269201ecd4eb49e03692cba4eced4799
Author: Daniel Machon <daniel.machon@microchip.com>

net: lan966x: add shutdown callback to stop FDMA on reboot

This commit adds a shutdown callback to the lan966x driver to mask FDMA
interrupts and disable RX and TX channels, preventing shared PCIe INTx
assertion during reboot.

In the newly added lan966x_shutdown() function, the network stack and
NAPI are not detached or disabled before shutting down the hardware.

Because network interfaces can remain active during system shutdown, could
concurrent packet transmissions invoke ndo_start_xmit and subsequently
lan966x_fdma_xmit()?

If lan966x_fdma_xmit() evaluates tx->activated as false after the shutdown
routine clears it, it appears it will unconditionally call
lan966x_fdma_tx_activate(). Will this directly write to hardware registers
to re-enable the TX DMA channel and unmask interrupts, effectively undoing
the shutdown sequence?

Similarly, could a concurrent NAPI poll finish and call napi_complete_done(),
rewriting 0xff to FDMA_INTR_DB_ENA and re-arming the doorbell interrupts?
Should the shutdown sequence call netif_device_detach() and napi_disable()
to ensure the device is fully quiesced?

While reviewing the device teardown paths, a pre-existing issue in
lan966x_remove() was noticed.

The driver calls ops->fdma_deinit() before unregistering the network
devices via lan966x_cleanup_ports(). Since fdma_deinit frees the TX
ring buffers, is it possible for the network stack to still invoke
ndo_start_xmit while the devices are registered?

Would this cause a use-after-free if lan966x_fdma_xmit() dereferences
and writes to the freed tx->dcbs_buf array? Should the network devices
be fully unregistered before their backing queue memory is destroyed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260428-lan966x-pci-fdma-v2-0-d3ec66e06202@microchip.com?part=8

  reply	other threads:[~2026-04-29 13:10 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-28 13:06 [PATCH net-next v2 00/13] net: lan966x: add support for PCIe FDMA Daniel Machon
2026-04-28 13:06 ` [PATCH net-next v2 01/13] MAINTAINERS: add FDMA library to Sparx5 SoC entry Daniel Machon
2026-04-28 13:06 ` [PATCH net-next v2 02/13] net: microchip: fdma: rename contiguous dataptr helpers Daniel Machon
2026-04-28 13:06 ` [PATCH net-next v2 03/13] net: microchip: fdma: add PCIe ATU support Daniel Machon
2026-04-28 13:06 ` [PATCH net-next v2 04/13] net: lan966x: add FDMA LLP register write helper Daniel Machon
2026-04-29 13:10   ` sashiko-bot
2026-04-28 13:06 ` [PATCH net-next v2 05/13] net: lan966x: export FDMA helpers for reuse Daniel Machon
2026-04-28 13:06 ` [PATCH net-next v2 06/13] net: lan966x: add FDMA ops dispatch for PCIe support Daniel Machon
2026-04-29 13:10   ` sashiko-bot
2026-04-28 13:06 ` [PATCH net-next v2 07/13] net: lan966x: clear FDMA interrupt stickies after switch reset Daniel Machon
2026-04-28 13:06 ` [PATCH net-next v2 08/13] net: lan966x: add shutdown callback to stop FDMA on reboot Daniel Machon
2026-04-29 13:10   ` sashiko-bot [this message]
2026-04-28 13:06 ` [PATCH net-next v2 09/13] net: lan966x: add PCIe FDMA support Daniel Machon
2026-04-29 13:10   ` sashiko-bot
2026-04-28 13:06 ` [PATCH net-next v2 10/13] net: lan966x: add PCIe FDMA MTU change support Daniel Machon
2026-04-29 13:10   ` sashiko-bot
2026-04-28 13:07 ` [PATCH net-next v2 11/13] net: lan966x: add PCIe FDMA XDP support Daniel Machon
2026-04-29 13:10   ` sashiko-bot
2026-04-28 13:07 ` [PATCH net-next v2 12/13] misc: lan966x-pci: dts: extend cpu reg to cover PCIE DBI space Daniel Machon
2026-04-28 13:07 ` [PATCH net-next v2 13/13] misc: lan966x-pci: dts: add fdma interrupt to overlay Daniel Machon
2026-04-28 16:07 ` [PATCH net-next v2 00/13] net: lan966x: add support for PCIe FDMA Herve Codina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260429131004.A671AC2BCB8@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel.machon@microchip.com \
    --cc=sashiko@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.