Re: [PATCH for-rc v2 5/8] RDMA/bnxt_re: Fix rdev lifetime races in suspend/resume/shutdown
From: sashiko-bot@kernel.org
Date: 2026-09-09 08:50:37
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: Pre-existing issues: - [High] The patch misses applying the Use-After-Free fix to `bnxt_re_remove()`, leaving the exact same race window open during device unbind or module unload. - [High] Use-After-Free during `bnxt_re_dev_init()` failure path due to internal resource teardown while `en_info->rdev` is still valid. - [High] Insufficient synchronization between `bnxt_re_update_en_info_rdev()` (`rtnl_lock`) and lockless RCU readers like `bnxt_re_async_notifier()`, leading to Use-After-Free. -- commit 1f3564eb53548d58fabdef6b5b5a607d2ac66cdf Author: Selvin Xavier [off-list ref] Subject: RDMA/bnxt_re: Fix rdev lifetime races in suspend/resume/shutdown This commit attempts to fix use-after-free windows in the suspend, resume, and shutdown paths by ensuring the rdev pointer is cleared before device resources are freed.
quoted hunk ↗ jump to hunk
diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c index ce72db1b4bc34..142bf9a8ca4a4 100644 --- a/drivers/infiniband/hw/bnxt_re/main.c +++ b/drivers/infiniband/hw/bnxt_re/main.c@@ -2427,11 +2427,14 @@ static int bnxt_re_add_device(struct auxiliary_device *adev, u8 op_type) bnxt_re_update_en_info_rdev(rdev, en_info, adev); rc = bnxt_re_dev_init(rdev, op_type); - if (rc) + if (rc) { + bnxt_re_update_en_info_rdev(NULL, en_info, adev); goto re_dev_dealloc; + }
[Severity: High] This is a pre-existing issue, but does this approach fully prevent the race during a device initialization failure? If bnxt_re_dev_init() fails internally after registering the netdev, it jumps to its internal fail label and calls bnxt_re_dev_uninit(). At this stage, en_info->rdev is still valid. If an async ULP callback triggers during this teardown, could it access partially destroyed resources before bnxt_re_dev_init() returns and the pointer is finally cleared here? [ ... ]
quoted hunk ↗ jump to hunk
@@ -2533,8 +2539,9 @@ static int bnxt_re_suspend(struct auxiliary_device *adev, pm_message_t state) ibdev_info(&rdev->ibdev, "%s: L2 driver notified to stop en_state 0x%lx", __func__, en_dev->en_state); - bnxt_re_remove_device(rdev, BNXT_RE_PRE_RECOVERY_REMOVE, adev); + + bnxt_re_update_en_info_rdev(NULL, en_info, adev); + bnxt_re_remove_device(rdev, BNXT_RE_PRE_RECOVERY_REMOVE, adev); mutex_unlock(&bnxt_re_mutex);
[Severity: High]
This is a pre-existing issue, but is there sufficient synchronization between
the pointer clearing and lockless RCU readers?
While bnxt_re_update_en_info_rdev() uses rtnl_lock() to clear the pointer,
readers like bnxt_re_async_notifier() read it under rcu_read_lock().
Because bnxt_re_remove_device() subsequently destroys the workqueue via
bnxt_re_uninit_dcb_wq() without waiting for synchronize_rcu(), could an
active concurrent RCU reader proceed to call queue_work() on the destroyed
workqueue?
Looking at the rest of the file, it appears a similar pattern exists in
bnxt_re_remove() which is not covered by this patch:
drivers/infiniband/hw/bnxt_re/main.c:bnxt_re_remove() {
...
rdev = en_info->rdev;
if (rdev)
bnxt_re_remove_device(rdev, BNXT_RE_COMPLETE_REMOVE, adev);
...
}
[Severity: High]
This is a pre-existing issue, but since the patch fixes the use-after-free
window during suspend, resume, and shutdown, does bnxt_re_remove() need the
same fix?
When the auxiliary device is removed, bnxt_re_remove() calls
bnxt_re_remove_device() to destroy resources without first clearing
en_info->rdev. Could an asynchronous L2 ULP callback fire concurrently and
access the freed memory?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909135244.122747-1-selvin.xavier@broadcom.com?part=5