Skip to content

Commit 33f016b

Browse files
orospkuba-moo
orosp
authored and
kuba-moo
committed
dpll: fix NULL deref in dpll_device_ops() during teardown race
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: 9431063 ("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>
1 parent 1e3d24f commit 33f016b

2 files changed

Lines changed: 67 additions & 16 deletions

File tree

‎drivers/dpll/dpll_core.c‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -876,19 +876,25 @@ int
876876
dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
877877
const struct dpll_pin_ops *ops, void *priv)
878878
{
879+
const struct dpll_device_ops *dev_ops;
879880
int ret;
880881

881882
if (WARN_ON(!ops) ||
882883
WARN_ON(!ops->state_on_dpll_get) ||
883884
WARN_ON(!ops->direction_get) ||
884-
WARN_ON(ops->measured_freq_get &&
885-
(!dpll_device_ops(dpll)->freq_monitor_get ||
886-
!dpll_device_ops(dpll)->freq_monitor_set)) ||
887885
WARN_ON(ops->supported_ffo && !ops->ffo_get))
888886
return -EINVAL;
889887

890888
mutex_lock(&dpll_lock);
891889

890+
dev_ops = dpll_device_ops(dpll);
891+
if (WARN_ON(ops->measured_freq_get &&
892+
(!dev_ops || !dev_ops->freq_monitor_get ||
893+
!dev_ops->freq_monitor_set))) {
894+
ret = -EINVAL;
895+
goto out_unlock;
896+
}
897+
892898
/*
893899
* For pins identified via firmware (pin->fwnode), allow registration
894900
* even if the pin's (module, clock_id) differs from the target DPLL.
@@ -1081,19 +1087,17 @@ EXPORT_SYMBOL_GPL(dpll_pin_ref_sync_pair_add);
10811087
static struct dpll_device_registration *
10821088
dpll_device_registration_first(struct dpll_device *dpll)
10831089
{
1084-
struct dpll_device_registration *reg;
1085-
1086-
reg = list_first_entry_or_null((struct list_head *)&dpll->registration_list,
1087-
struct dpll_device_registration, list);
1088-
WARN_ON(!reg);
1089-
return reg;
1090+
return list_first_entry_or_null((struct list_head *)&dpll->registration_list,
1091+
struct dpll_device_registration, list);
10901092
}
10911093

10921094
void *dpll_priv(struct dpll_device *dpll)
10931095
{
10941096
struct dpll_device_registration *reg;
10951097

10961098
reg = dpll_device_registration_first(dpll);
1099+
if (!reg)
1100+
return NULL;
10971101
return reg->priv;
10981102
}
10991103

@@ -1102,6 +1106,8 @@ const struct dpll_device_ops *dpll_device_ops(struct dpll_device *dpll)
11021106
struct dpll_device_registration *reg;
11031107

11041108
reg = dpll_device_registration_first(dpll);
1109+
if (!reg)
1110+
return NULL;
11051111
return reg->ops;
11061112
}
11071113

‎drivers/dpll/dpll_netlink.c‎

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,22 @@ static bool dpll_pin_available(struct dpll_pin *pin)
6666
return false;
6767
}
6868

69+
static bool dpll_device_registered(struct dpll_device *dpll)
70+
{
71+
return dpll_device_ops(dpll);
72+
}
73+
74+
static struct dpll_pin_ref *dpll_pin_first_registered_ref(struct dpll_pin *pin)
75+
{
76+
struct dpll_pin_ref *ref;
77+
unsigned long i;
78+
79+
xa_for_each(&pin->dpll_refs, i, ref)
80+
if (dpll_device_registered(ref->dpll))
81+
return ref;
82+
return NULL;
83+
}
84+
6985
/**
7086
* dpll_msg_add_pin_handle - attach pin handle attribute to a given message
7187
* @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,
656672
int ret;
657673

658674
xa_for_each(&pin->dpll_refs, index, ref) {
675+
if (!dpll_device_registered(ref->dpll))
676+
continue;
659677
attr = nla_nest_start(msg, DPLL_A_PIN_PARENT_DEVICE);
660678
if (!attr)
661679
return -EMSGSIZE;
@@ -700,9 +718,10 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
700718
int ret;
701719

702720
ref = dpll_pin_own_dpll_ref_first(pin);
721+
if (!ref || !dpll_device_registered(ref->dpll))
722+
ref = dpll_pin_first_registered_ref(pin);
703723
if (!ref)
704-
ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
705-
ASSERT_NOT_NULL(ref);
724+
return -ENODEV;
706725

707726
ret = dpll_msg_add_pin_handle(msg, pin);
708727
if (ret)
@@ -1091,6 +1110,8 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
10911110
}
10921111

10931112
xa_for_each(&pin->dpll_refs, i, ref) {
1113+
if (!dpll_device_registered(ref->dpll))
1114+
continue;
10941115
ops = dpll_pin_ops(ref);
10951116
if ((!ops->frequency_set || !ops->frequency_get) &&
10961117
ref->dpll->module == pin->module &&
@@ -1101,7 +1122,7 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
11011122
}
11021123
}
11031124
ref = dpll_pin_own_dpll_ref_first(pin);
1104-
if (!ref) {
1125+
if (!ref || !dpll_device_registered(ref->dpll)) {
11051126
NL_SET_ERR_MSG(extack, "pin owner dpll not found");
11061127
return -ENODEV;
11071128
}
@@ -1117,6 +1138,8 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
11171138
return 0;
11181139

11191140
xa_for_each(&pin->dpll_refs, i, ref) {
1141+
if (!dpll_device_registered(ref->dpll))
1142+
continue;
11201143
ops = dpll_pin_ops(ref);
11211144
if (!ops->frequency_set)
11221145
continue;
@@ -1138,6 +1161,8 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
11381161
xa_for_each(&pin->dpll_refs, i, ref) {
11391162
if (ref == failed)
11401163
break;
1164+
if (!dpll_device_registered(ref->dpll))
1165+
continue;
11411166
ops = dpll_pin_ops(ref);
11421167
if (!ops->frequency_set)
11431168
continue;
@@ -1163,6 +1188,8 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
11631188
int ret;
11641189

11651190
xa_for_each(&pin->dpll_refs, i, ref) {
1191+
if (!dpll_device_registered(ref->dpll))
1192+
continue;
11661193
ops = dpll_pin_ops(ref);
11671194
if ((!ops->esync_set || !ops->esync_get) &&
11681195
ref->dpll->module == pin->module &&
@@ -1173,7 +1200,7 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
11731200
}
11741201
}
11751202
ref = dpll_pin_own_dpll_ref_first(pin);
1176-
if (!ref) {
1203+
if (!ref || !dpll_device_registered(ref->dpll)) {
11771204
NL_SET_ERR_MSG(extack, "pin owner dpll not found");
11781205
return -ENODEV;
11791206
}
@@ -1199,6 +1226,8 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
11991226
xa_for_each(&pin->dpll_refs, i, ref) {
12001227
void *pin_dpll_priv;
12011228

1229+
if (!dpll_device_registered(ref->dpll))
1230+
continue;
12021231
ops = dpll_pin_ops(ref);
12031232
if (!ops->esync_set)
12041233
continue;
@@ -1224,6 +1253,8 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
12241253

12251254
if (ref == failed)
12261255
break;
1256+
if (!dpll_device_registered(ref->dpll))
1257+
continue;
12271258
ops = dpll_pin_ops(ref);
12281259
if (!ops->esync_set)
12291260
continue;
@@ -1262,7 +1293,7 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
12621293
return -EINVAL;
12631294
}
12641295
ref = dpll_pin_own_dpll_ref_first(pin);
1265-
if (!ref) {
1296+
if (!ref || !dpll_device_registered(ref->dpll)) {
12661297
NL_SET_ERR_MSG(extack, "pin owner dpll not found");
12671298
return -ENODEV;
12681299
}
@@ -1283,6 +1314,8 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
12831314
if (state == old_state)
12841315
return 0;
12851316
xa_for_each(&pin->dpll_refs, i, ref) {
1317+
if (!dpll_device_registered(ref->dpll))
1318+
continue;
12861319
ops = dpll_pin_ops(ref);
12871320
if (!ops->ref_sync_set)
12881321
continue;
@@ -1307,6 +1340,8 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
13071340
xa_for_each(&pin->dpll_refs, i, ref) {
13081341
if (ref == failed)
13091342
break;
1343+
if (!dpll_device_registered(ref->dpll))
1344+
continue;
13101345
ops = dpll_pin_ops(ref);
13111346
if (!ops->ref_sync_set)
13121347
continue;
@@ -1500,6 +1535,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
15001535
}
15011536

15021537
xa_for_each(&pin->dpll_refs, i, ref) {
1538+
if (!dpll_device_registered(ref->dpll))
1539+
continue;
15031540
ops = dpll_pin_ops(ref);
15041541
if ((!ops->phase_adjust_set || !ops->phase_adjust_get) &&
15051542
ref->dpll->module == pin->module &&
@@ -1509,7 +1546,7 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
15091546
}
15101547
}
15111548
ref = dpll_pin_own_dpll_ref_first(pin);
1512-
if (!ref) {
1549+
if (!ref || !dpll_device_registered(ref->dpll)) {
15131550
NL_SET_ERR_MSG(extack, "pin owner dpll not found");
15141551
return -ENODEV;
15151552
}
@@ -1526,6 +1563,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
15261563
return 0;
15271564

15281565
xa_for_each(&pin->dpll_refs, i, ref) {
1566+
if (!dpll_device_registered(ref->dpll))
1567+
continue;
15291568
ops = dpll_pin_ops(ref);
15301569
if (!ops->phase_adjust_set)
15311570
continue;
@@ -1550,6 +1589,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
15501589
xa_for_each(&pin->dpll_refs, i, ref) {
15511590
if (ref == failed)
15521591
break;
1592+
if (!dpll_device_registered(ref->dpll))
1593+
continue;
15531594
ops = dpll_pin_ops(ref);
15541595
if (!ops->phase_adjust_set)
15551596
continue;
@@ -1581,7 +1622,7 @@ dpll_pin_parent_device_set(struct dpll_pin *pin, struct nlattr *parent_nest,
15811622
return -EINVAL;
15821623
}
15831624
pdpll_idx = nla_get_u32(tb[DPLL_A_PIN_PARENT_ID]);
1584-
dpll = xa_load(&dpll_device_xa, pdpll_idx);
1625+
dpll = dpll_device_get_by_id(pdpll_idx);
15851626
if (!dpll) {
15861627
NL_SET_ERR_MSG(extack, "parent device not found");
15871628
return -EINVAL;
@@ -1873,6 +1914,10 @@ int dpll_nl_pin_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
18731914
ret = dpll_cmd_pin_get_one(skb, pin, cb->extack);
18741915
if (ret) {
18751916
genlmsg_cancel(skb, hdr);
1917+
if (ret == -ENODEV) {
1918+
ret = 0;
1919+
continue;
1920+
}
18761921
break;
18771922
}
18781923
genlmsg_end(skb, hdr);

0 commit comments

Comments
 (0)