[PATCH net-next v3 0/8] ibmveth: fix hangs, use-after-frees and netpoll races

[Mingming Cao]

Hi,

Eight fixes for serious bugs in the ibmveth driver: two hang the
system, four are use-after-frees, one corrupts memory, and one makes
ethtool -L report success when it failed.

  1. Netpoll races with RX replenish (memory corruption, NULL
     dereference): remove ndo_poll_controller, as was done for
     ibmvnic, skip RX replenish in netpoll's budget-0 poll, and
     enable NAPI only once the RX resources exist.
     Fixes: 6b4223748895, bea3348eef27

  2. Hang after a failed internal reopen: the next close() waits in
     napi_disable() forever with RTNL held, and only a reboot
     recovers. Skip close() when open() did not succeed.
     Fixes: 860f242eb534

  3. Use-after-free in remove(): a reset queued from NAPI could run
     on the freed adapter. Disable the reset work first.
     Fixes: 2c91e2319ed9

  4. Fixes the RX poll when the correlator is bad. Poll spun on that
     slot until RCU stalled, and an inactive pool dereferenced NULL.
     Skipping the slot and then leaving poll could also queue the
     napi twice. A frame that does not fit its buffer was copied
     past the end. Move past the bad slot and stay in poll, and
     drop a frame that does not fit.
     Fixes: 2c91e2319ed9, 860f242eb534

  5. Use-after-free after a failed probe: the pool kobjects stay in
     sysfs after the adapter is freed. Put them, as remove() does,
     and give them a release() that probe and remove() wait for, so
     CONFIG_DEBUG_KOBJECT_RELEASE cannot free them early either.
     Fixes: 860f242eb534

  6. ethtool -L returns 0 when it cannot allocate the new TX queues.
     Return the allocation error.
     Fixes: 10c2aba89cc0

  7. Use-after-free of TX buffers: close() frees them without
     waiting for a running transmit. It is called directly for MTU,
     offload and buffer pool changes, and through dev_close(), which
     does not wait either with a noqueue qdisc. Use
     netif_tx_disable().
     Fixes: d6832ca48d8a

  8. Use-after-free of the RX queue: napi_disable() returns before
     the poll has finished, and the rest of the poll re-enables the
     interrupt and reads the RX queue that close() then frees. Wait
     with synchronize_net().
     Fixes: bea3348eef27

All were found by AI-assisted review of the ibmveth multi-queue
RX series [1]. Landing them first also shrinks that series, which
then only extends this handling per queue.

Testing: the new and extended KUnit cases in patch 4 fail on the
unfixed driver and pass on qemu pseries (ppc64le). On a POWER10
LPAR: netconsole under printk and ping floods, MTU and buffer pool
changes under traffic, a forced open() failure followed by down and
up, unbind/bind under traffic, a forced register_netdev() failure in
probe, ethtool -L with a forced TX buffer allocation failure, and
rapid link down/up and MTU cycles under a ping flood for patch 8.
The forced failures used test-only module parameters that are not
part of this series.

The triggers are rare and there are no field reports, so the series
targets net-next. It is based on net-next d8674294aefe ("Merge
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net"), which
includes the two open() error-path fixes (af0524bf4ce1,
84bec0bf0352). Patch 2 explains how it relates to them. It also
applies cleanly to net. All carry Fixes: tags; I am happy to
repost against net, or add Cc: stable, if you prefer.

Changes in v3:

>From the Sashiko review of v2:
- Patch 4: on a bad RX slot, stay in the poll loop and return
  budget - 1 after napi_schedule() has queued the napi. Breaking
  out was queuing it twice.
- Patch 4: drop a frame that does not fit its buffer, instead of
  copying past the end.
- Patch 4: also drop a frame shorter than an Ethernet header, and
  take the pool for the buffer bound from the correlator that was
  validated, read once.
- Patch 2: commit message: the two open() fixes are now in net-next.

v2: https://lore.kernel.org/netdev/cover.1791178212.git.mmc@xxxxxxxxxxxxx/

Changes in v2:

>From the Sashiko review of v1:
- Patch 5: give the pool kobjects a release() and wait for it before
  free_netdev() in probe and remove(), which closes the
  CONFIG_DEBUG_KOBJECT_RELEASE window.
- New patch 8: wait for the poll to return before close() frees the
  RX queue (raised on patch 1 as a pre-existing bug).

Other small changes:
- Patch 1: also skip RX replenish in netpoll's budget-0 poll.
- Patch 4: count rx_dropped when recycling an invalid buffer fails.
- Commit messages: say how each bug was found and tested, with small
  clarifications in patches 2 and 7.

Rebased on current net-next.

v1: https://lore.kernel.org/netdev/cover.1790991039.git.mmc@xxxxxxxxxxxxx/

[1] https://lore.kernel.org/netdev/cover.1790319558.git.mmc@xxxxxxxxxxxxx/

Thanks,
Mingming

Mingming Cao (8):
  ibmveth: fix netpoll races with RX replenish
  ibmveth: do not close twice after a failed reopen
  ibmveth: disable the reset work before unregister in remove
  ibmveth: step past bad RX correlators instead of spinning or oopsing
  ibmveth: release the pool kobjects when probe fails
  ibmveth: return the error when set_channels cannot add TX queues
  ibmveth: wait for in-flight transmits in ibmveth_close()
  ibmveth: wait for the RX poll to return before freeing the RX queue

 drivers/net/ethernet/ibm/ibmveth.c | 364 +++++++++++++++++++++++------
 drivers/net/ethernet/ibm/ibmveth.h |   5 +
 2 files changed, 296 insertions(+), 73 deletions(-)


base-commit: d8674294aefef02266c4d47ad10131f1bffbe534
-- 
2.50.1 (Apple Git-155)