mirrors/linux
2
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2026-08-28 09:53:15 -04:00
Code Issues Packages Projects Releases Wiki Activity

dpll: fix NULL deref in dpll_device_ops() during teardown race

Browse Source 
When the last owner of a dpll device unregisters while a foreign driver
still holds a pin on it via dpll_pin_on_pin_register(), the dpll object
stays alive with an empty registration list. A pin notification queued
before the unregister (e.g. ice reacting to zl3073x_i2c removal) then
walks pin->dpll_refs into dpll_device_ops(), which trips the WARN_ON and
dereferences the missing registration. dpll_lock cannot help because the
notification work was queued before the unregistering driver took the
lock.

Treat the empty registration list as a legitimate transient state. Make
dpll_priv() and dpll_device_ops() return NULL in that case and make
every pin netlink path that resolves a device from a pin skip such
dplls. dpll_cmd_pin_get_one() picks a ref with a live registration and
returns -ENODEV when there is none, the pin dumpit skips such a pin
instead of aborting the dump, dpll_msg_add_pin_dplls() and the
frequency, esync, reference sync and phase adjust set paths skip dead
refs, and dpll_pin_parent_device_set() validates the parent with
dpll_device_get_by_id(). dpll_pin_register() is the last caller that
dereferenced the device ops without a check, so move its frequency
monitor validation under dpll_lock and tolerate a missing registration
there as well.

The empty registration list is equivalent to a cleared DPLL_REGISTERED
mark, both transitions happen under dpll_lock in dpll_device_register()
and dpll_device_unregister(). A pin notification for a pin whose dplls
are all gone is now dropped with -ENODEV instead of crashing, all
callers in the core ignore that return value.

 WARNING: drivers/dpll/dpll_core.c:1092 at dpll_device_ops+0x24/0x40,
 CPU#83: kworker/u576:3/23471
 Modules linked in: ... ice ... zl3073x_i2c(-) ... zl3073x ...
 Workqueue: ice_dpll_wq ice_dpll_pin_notify_work [ice]
 RIP: 0010:dpll_device_ops+0x24/0x40
 Call Trace:
  <TASK>
  dpll_cmd_pin_get_one+0x336/0x520
  dpll_pin_event_send+0x82/0x140
  dpll_pin_on_pin_unregister+0xbb/0x160
  ice_dpll_pin_notify_work+0x1bc/0x1f0 [ice]
  process_one_work+0x19e/0x370
  worker_thread+0x1a6/0x310
  kthread+0xe4/0x120
  ret_from_fork+0x1a1/0x270
  ret_from_fork_asm+0x1a/0x30
  </TASK>
 ---[ end trace 0000000000000000 ]---
 BUG: kernel NULL pointer dereference, address: 0000000000000010
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page

Fixes: 9431063ad3 ("dpll: core: Add DPLL framework base functions")
Signed-off-by: Petr Oros <poros@redhat.com>
Tested-by: Ivan Vecera <ivecera@redhat.com>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Link: https://patch.msgid.link/20260813140817.1051388-1-poros@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Petr Oros
2026-08-13 16:08:17 +02:00
committed by Jakub Kicinski
parent 1e3d24fc42
commit 33f016b23a
2 changed files with 67 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
drivers/dpll/dpll_core.c
View File

@@ -876,19 +876,25 @@ int
dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
const struct dpll_pin_ops *ops, void *priv)
{
const struct dpll_device_ops *dev_ops;
int ret;
if (WARN_ON(!ops) ||
WARN_ON(!ops->state_on_dpll_get) ||
WARN_ON(!ops->direction_get) ||
WARN_ON(ops->measured_freq_get &&
(!dpll_device_ops(dpll)->freq_monitor_get ||
!dpll_device_ops(dpll)->freq_monitor_set)) ||
WARN_ON(ops->supported_ffo && !ops->ffo_get))
return -EINVAL;
mutex_lock(&dpll_lock);
dev_ops = dpll_device_ops(dpll);
if (WARN_ON(ops->measured_freq_get &&
(!dev_ops || !dev_ops->freq_monitor_get ||
!dev_ops->freq_monitor_set))) {
ret = -EINVAL;
goto out_unlock;
}
/*
* For pins identified via firmware (pin->fwnode), allow registration
* even if the pin's (module, clock_id) differs from the target DPLL.
@@ -1081,12 +1087,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)
@@ -1094,6 +1096,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;
}
@@ -1102,6 +1106,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;
}

59
drivers/dpll/dpll_netlink.c
View File

@@ -66,6 +66,22 @@ static bool dpll_pin_available(struct dpll_pin *pin)
return false;
}
static bool dpll_device_registered(struct dpll_device *dpll)
{
return dpll_device_ops(dpll);
}
static struct dpll_pin_ref *dpll_pin_first_registered_ref(struct dpll_pin *pin)
{
struct dpll_pin_ref *ref;
unsigned long i;
xa_for_each(&pin->dpll_refs, i, ref)
if (dpll_device_registered(ref->dpll))
return ref;
return NULL;
}
/**
* dpll_msg_add_pin_handle - attach pin handle attribute to a given message
* @msg: pointer to sk_buff message to attach a pin handle
@@ -656,6 +672,8 @@ dpll_msg_add_pin_dplls(struct sk_buff *msg, struct dpll_pin *pin,
int ret;
xa_for_each(&pin->dpll_refs, index, ref) {
if (!dpll_device_registered(ref->dpll))
continue;
attr = nla_nest_start(msg, DPLL_A_PIN_PARENT_DEVICE);
if (!attr)
return -EMSGSIZE;
@@ -700,9 +718,10 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
int ret;
ref = dpll_pin_own_dpll_ref_first(pin);
if (!ref || !dpll_device_registered(ref->dpll))
ref = dpll_pin_first_registered_ref(pin);
if (!ref)
ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
ASSERT_NOT_NULL(ref);
return -ENODEV;
ret = dpll_msg_add_pin_handle(msg, pin);
if (ret)
@@ -1091,6 +1110,8 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
}
xa_for_each(&pin->dpll_refs, i, ref) {
if (!dpll_device_registered(ref->dpll))
continue;
ops = dpll_pin_ops(ref);
if ((!ops->frequency_set || !ops->frequency_get) &&
ref->dpll->module == pin->module &&
@@ -1101,7 +1122,7 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
}
}
ref = dpll_pin_own_dpll_ref_first(pin);
if (!ref) {
if (!ref || !dpll_device_registered(ref->dpll)) {
NL_SET_ERR_MSG(extack, "pin owner dpll not found");
return -ENODEV;
}
@@ -1117,6 +1138,8 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
return 0;
xa_for_each(&pin->dpll_refs, i, ref) {
if (!dpll_device_registered(ref->dpll))
continue;
ops = dpll_pin_ops(ref);
if (!ops->frequency_set)
continue;
@@ -1138,6 +1161,8 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
xa_for_each(&pin->dpll_refs, i, ref) {
if (ref == failed)
break;
if (!dpll_device_registered(ref->dpll))
continue;
ops = dpll_pin_ops(ref);
if (!ops->frequency_set)
continue;
@@ -1163,6 +1188,8 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
int ret;
xa_for_each(&pin->dpll_refs, i, ref) {
if (!dpll_device_registered(ref->dpll))
continue;
ops = dpll_pin_ops(ref);
if ((!ops->esync_set || !ops->esync_get) &&
ref->dpll->module == pin->module &&
@@ -1173,7 +1200,7 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
}
}
ref = dpll_pin_own_dpll_ref_first(pin);
if (!ref) {
if (!ref || !dpll_device_registered(ref->dpll)) {
NL_SET_ERR_MSG(extack, "pin owner dpll not found");
return -ENODEV;
}
@@ -1199,6 +1226,8 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
xa_for_each(&pin->dpll_refs, i, ref) {
void *pin_dpll_priv;
if (!dpll_device_registered(ref->dpll))
continue;
ops = dpll_pin_ops(ref);
if (!ops->esync_set)
continue;
@@ -1224,6 +1253,8 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
if (ref == failed)
break;
if (!dpll_device_registered(ref->dpll))
continue;
ops = dpll_pin_ops(ref);
if (!ops->esync_set)
continue;
@@ -1262,7 +1293,7 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
return -EINVAL;
}
ref = dpll_pin_own_dpll_ref_first(pin);
if (!ref) {
if (!ref || !dpll_device_registered(ref->dpll)) {
NL_SET_ERR_MSG(extack, "pin owner dpll not found");
return -ENODEV;
}
@@ -1283,6 +1314,8 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
if (state == old_state)
return 0;
xa_for_each(&pin->dpll_refs, i, ref) {
if (!dpll_device_registered(ref->dpll))
continue;
ops = dpll_pin_ops(ref);
if (!ops->ref_sync_set)
continue;
@@ -1307,6 +1340,8 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
xa_for_each(&pin->dpll_refs, i, ref) {
if (ref == failed)
break;
if (!dpll_device_registered(ref->dpll))
continue;
ops = dpll_pin_ops(ref);
if (!ops->ref_sync_set)
continue;
@@ -1500,6 +1535,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
}
xa_for_each(&pin->dpll_refs, i, ref) {
if (!dpll_device_registered(ref->dpll))
continue;
ops = dpll_pin_ops(ref);
if ((!ops->phase_adjust_set || !ops->phase_adjust_get) &&
ref->dpll->module == pin->module &&
@@ -1509,7 +1546,7 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
}
}
ref = dpll_pin_own_dpll_ref_first(pin);
if (!ref) {
if (!ref || !dpll_device_registered(ref->dpll)) {
NL_SET_ERR_MSG(extack, "pin owner dpll not found");
return -ENODEV;
}
@@ -1526,6 +1563,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
return 0;
xa_for_each(&pin->dpll_refs, i, ref) {
if (!dpll_device_registered(ref->dpll))
continue;
ops = dpll_pin_ops(ref);
if (!ops->phase_adjust_set)
continue;
@@ -1550,6 +1589,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
xa_for_each(&pin->dpll_refs, i, ref) {
if (ref == failed)
break;
if (!dpll_device_registered(ref->dpll))
continue;
ops = dpll_pin_ops(ref);
if (!ops->phase_adjust_set)
continue;
@@ -1581,7 +1622,7 @@ dpll_pin_parent_device_set(struct dpll_pin *pin, struct nlattr *parent_nest,
return -EINVAL;
}
pdpll_idx = nla_get_u32(tb[DPLL_A_PIN_PARENT_ID]);
dpll = xa_load(&dpll_device_xa, pdpll_idx);
dpll = dpll_device_get_by_id(pdpll_idx);
if (!dpll) {
NL_SET_ERR_MSG(extack, "parent device not found");
return -EINVAL;
@@ -1873,6 +1914,10 @@ int dpll_nl_pin_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
ret = dpll_cmd_pin_get_one(skb, pin, cb->extack);
if (ret) {
genlmsg_cancel(skb, hdr);
if (ret == -ENODEV) {
ret = 0;
continue;
}
break;
}
genlmsg_end(skb, hdr);