When firmware delivers OCTEP_VDPA_DEV_DEL_EVENT, octep_event_work()
checks if mgmt_dev->status is OCTEP_VDPA_DEV_STATUS_ADDED, but does
not claim ownership before calling octep_vdpa_dev_del(). Meanwhile,
userspace management deletion via octep_vdpa_dev_del() unconditionally
calls _vdpa_unregister_device() and marks status as REMOVED.
If a queued hardware DEL event checks the ADDED state and the management
path subsequently deletes the same oct_vdpa, the worker proceeds to call
octep_vdpa_dev_del() on the already unregistered and freed device. This
results in _vdpa_unregister_device() being invoked twice on the same
object, triggering a use-after-free or double release.
Fix this race by:
1. Atomically claiming deletion ownership in octep_vdpa_dev_del() using
atomic_cmpxchg() from ADDED to REMOVED, and returning early if the
device was already claimed for deletion.
2. Clearing mgmt_dev->oct_vdpa upon unregistration and on dev_add
failure so dangling pointers are not referenced.
3. Reading mgmt_dev->oct_vdpa safely in octep_event_work() before
invoking octep_vdpa_dev_del().
4. Canceling event work and ensuring child devices are unregistered
before marking the management device UNINIT in octep_vdpa_remove_vf().
Fixes: a5786561649a ("vdpa/octeon_ep: Add vDPA device event handling for
firmware notifications")
Cc: [email protected]
Signed-off-by: Yuho Choi <[email protected]>
---
drivers/vdpa/octeon_ep/octep_vdpa_main.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
index 85a3d35ea1e4..e29647bcf196 100644
--- a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
+++ b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
@@ -523,14 +523,16 @@ static void octep_vdpa_remove_vf(struct pci_dev *pdev)
int status;
oct_hw = &mgmt_dev->oct_hw;
- status = atomic_read(&mgmt_dev->status);
- atomic_set(&mgmt_dev->status, OCTEP_VDPA_DEV_STATUS_UNINIT);
-
cancel_work_sync(&mgmt_dev->setup_task);
+ cancel_work_sync(&mgmt_dev->event_wk.work);
+
+ status = atomic_read(&mgmt_dev->status);
if ((status == OCTEP_VDPA_DEV_STATUS_READY) || (status ==
OCTEP_VDPA_DEV_STATUS_ADDED) ||
(status == OCTEP_VDPA_DEV_STATUS_REMOVED))
vdpa_mgmtdev_unregister(&mgmt_dev->mdev);
+ atomic_set(&mgmt_dev->status, OCTEP_VDPA_DEV_STATUS_UNINIT);
+
if (oct_hw->base[OCTEP_HW_CAPS_BAR])
octep_iounmap_region(pdev, oct_hw->base, OCTEP_HW_CAPS_BAR);
@@ -612,6 +614,7 @@ static int octep_vdpa_dev_add(struct vdpa_mgmt_dev *mdev,
const char *name,
return 0;
vdpa_dev_put:
+ mgmt_dev->oct_vdpa = NULL;
put_device(&oct_vdpa->vdpa.dev);
return ret;
}
@@ -619,8 +622,13 @@ static int octep_vdpa_dev_add(struct vdpa_mgmt_dev *mdev,
const char *name,
static void octep_vdpa_dev_del(struct vdpa_mgmt_dev *mdev, struct vdpa_device
*vdpa_dev)
{
struct octep_vdpa_mgmt_dev *mgmt_dev = container_of(mdev, struct
octep_vdpa_mgmt_dev, mdev);
+
+ if (atomic_cmpxchg(&mgmt_dev->status, OCTEP_VDPA_DEV_STATUS_ADDED,
+ OCTEP_VDPA_DEV_STATUS_REMOVED) !=
OCTEP_VDPA_DEV_STATUS_ADDED)
+ return;
+
_vdpa_unregister_device(vdpa_dev);
- atomic_set(&mgmt_dev->status, OCTEP_VDPA_DEV_STATUS_REMOVED);
+ mgmt_dev->oct_vdpa = NULL;
}
static const struct vdpa_mgmtdev_ops octep_vdpa_mgmt_dev_ops = {
@@ -653,6 +661,7 @@ static void octep_event_work(struct work_struct *work)
u8 event = readb(addr + OCTEP_VF_EVENT_REG(0));
struct vdpa_dev_set_config config = {0};
char name[OCTEP_VDPA_NAME_BUFSIZE];
+ struct octep_vdpa *oct_vdpa;
int ret = 0;
switch (event) {
@@ -663,8 +672,9 @@ static void octep_event_work(struct work_struct *work)
}
break;
case OCTEP_VDPA_DEV_DEL_EVENT:
- if (atomic_read(&mgmt_dev->status) ==
OCTEP_VDPA_DEV_STATUS_ADDED)
- octep_vdpa_dev_del(&mgmt_dev->mdev,
&mgmt_dev->oct_vdpa->vdpa);
+ oct_vdpa = READ_ONCE(mgmt_dev->oct_vdpa);
+ if (atomic_read(&mgmt_dev->status) ==
OCTEP_VDPA_DEV_STATUS_ADDED && oct_vdpa)
+ octep_vdpa_dev_del(&mgmt_dev->mdev, &oct_vdpa->vdpa);
break;
default:
break;
--
2.43.0
