When a dpll device driver (zl3073x) is unloaded via rmmod while a
foreign driver (ice) still holds pins on it via
dpll_pin_on_pin_register(), dpll_device_unregister() empties
dpll->registration_list but the dpll object stays alive on ice's
refcount. A subsequent PIN_DELETED
notification on ice's pin reaches dpll_cmd_pin_get_one() which calls
dpll_device_ops() on the still-indexed but half-dead dpll;
dpll_device_registration_first() returns NULL and the accessor
dereferences ops at offset 0x10 (struct dpll_device_registration::ops):
CPU A (zl3073x rmmod) CPU B (ice workqueue)
--------------------- ---------------------
dpll_device_unregister():
drop reg from
dpll->registration_list
dpll_pin_on_pin_unregister
dpll_pin_delete_ntf
dpll_cmd_pin_get_one
dpll_device_ops()
registration_first() = NULL
NULL deref @ 0x10
WARNING: CPU: 27 PID: 972 at drivers/dpll/dpll_core.c:1072 dpll_device_ops+0x20/0x30
Workqueue: ice_dpll_wq ice_dpll_pin_notify_work [ice]
RIP: 0010:dpll_device_ops+0x20/0x30
Call Trace:
? __warn+0x84/0x140
? dpll_device_ops+0x20/0x30
? report_bug+0x16b/0x180
? handle_bug+0x3c/0x70
? exc_invalid_op+0x14/0x70
? asm_exc_invalid_op+0x16/0x20
? dpll_device_ops+0x20/0x30
dpll_cmd_pin_get_one+0x303/0x490
dpll_pin_event_send+0x93/0x140
dpll_pin_on_pin_unregister+0x45/0xe0
ice_dpll_pin_notify_work+0x7b/0x150 [ice]
process_one_work+0x188/0x3b0
worker_thread+0x2ef/0x410
kthread+0x122/0x240
ret_from_fork+0x28/0x50
---[ end trace 0000000000000000 ]---
BUG: kernel NULL pointer dereference, address: 0000000000000010
RIP: 0010:dpll_device_ops+0x24/0x30
Call Trace:
dpll_cmd_pin_get_one+0x303/0x490
dpll_pin_event_send+0x93/0x140
dpll_pin_on_pin_unregister+0x45/0xe0
ice_dpll_pin_notify_work+0x7b/0x150 [ice]
process_one_work+0x188/0x3b0
worker_thread+0x2ef/0x410
kthread+0x122/0x240
ret_from_fork+0x28/0x50
dpll_lock serializes the two paths but cannot reorder them: ice's work
was queued before zl3073x took the lock. "Empty registration_list on a
still-indexed dpll" is a legitimate transient state caused by
peer-driver refcounting; the accessors must tolerate it.
Drop the WARN_ON in dpll_device_registration_first(), make dpll_priv()
and dpll_device_ops() return NULL on the empty-list state, and bail
out cleanly in dpll_cmd_pin_get_one() when the first dpll the pin
references is already gone.
In-tree doit/dumpit callers of dpll_priv() / dpll_device_ops() obtain
their dpll via dpll_pre_doit() -> dpll_device_get_by_id(), which only
returns DPLL_REGISTERED devices; dpll_device_unregister() clears that
mark only after observing list_empty(&dpll->registration_list), so
those callers cannot see the new NULL return. The only path that can
is dpll_cmd_pin_get_one() reached from dpll_pin_event_send(), gated
here.
Fixes: 9431063ad323 ("dpll: core: Add DPLL framework base functions")
Signed-off-by: Petr Oros
---
drivers/dpll/dpll_core.c | 12 ++++++------
drivers/dpll/dpll_netlink.c | 6 ++++++
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index cbb635db43210f..4a058b46c69d4f 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -1060,12 +1060,8 @@ EXPORT_SYMBOL_GPL(dpll_pin_ref_sync_pair_add);
static struct dpll_device_registration *
dpll_device_registration_first(struct dpll_device *dpll)
{
- struct dpll_device_registration *reg;
-
- reg = list_first_entry_or_null((struct list_head *)&dpll->registration_list,
- struct dpll_device_registration, list);
- WARN_ON(!reg);
- return reg;
+ return list_first_entry_or_null((struct list_head *)&dpll->registration_list,
+ struct dpll_device_registration, list);
}
void *dpll_priv(struct dpll_device *dpll)
@@ -1073,6 +1069,8 @@ void *dpll_priv(struct dpll_device *dpll)
struct dpll_device_registration *reg;
reg = dpll_device_registration_first(dpll);
+ if (!reg)
+ return NULL;
return reg->priv;
}
@@ -1081,6 +1079,8 @@ const struct dpll_device_ops *dpll_device_ops(struct dpll_device *dpll)
struct dpll_device_registration *reg;
reg = dpll_device_registration_first(dpll);
+ if (!reg)
+ return NULL;
return reg->ops;
}
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index 0ff1658c2dc1ba..8e7e61982b867c 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -682,6 +682,12 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
ASSERT_NOT_NULL(ref);
+ /* The first dpll the pin references may be torn down while still
+ * pinned by foreign-driver refs; drop the notification cleanly.
+ */
+ if (!dpll_device_ops(ref->dpll))
+ return -ENODEV;
+
ret = dpll_msg_add_pin_handle(msg, pin);
if (ret)
return ret;
--
2.53.0