Skip to content

Commit 94fa82b

Browse files
davem330davem330
committed
Merge branch 'dpll-fixes'
Arkadiusz Kubalewski says: ==================== dpll: fix unordered unbind/bind registerer issues Fix issues when performing unordered unbind/bind of a kernel modules which are using a dpll device with DPLL_PIN_TYPE_MUX pins. Currently only serialized bind/unbind of such use case works, fix the issues and allow for unserialized kernel module bind order. The issues are observed on the ice driver, i.e., $ echo 0000:af:00.0 > /sys/bus/pci/drivers/ice/unbind $ echo 0000:af:00.1 > /sys/bus/pci/drivers/ice/unbind results in: ice 0000:af:00.0: Removed PTP clock BUG: kernel NULL pointer dereference, address: 0000000000000010 PF: supervisor read access in kernel mode PF: error_code(0x0000) - not-present page PGD 0 P4D 0 Oops: 0000 [#1] PREEMPT SMP PTI CPU: 7 PID: 71848 Comm: bash Kdump: loaded Not tainted 6.6.0-rc5_next-queue_19th-Oct-2023-01625-g039e5d15e451 #1 Hardware name: Intel Corporation S2600STB/S2600STB, BIOS SE5C620.86B.02.01.0008.031920191559 03/19/2019 RIP: 0010:ice_dpll_rclk_state_on_pin_get+0x2f/0x90 [ice] Code: 41 57 4d 89 cf 41 56 41 55 4d 89 c5 41 54 55 48 89 f5 53 4c 8b 66 08 48 89 cb 4d 8d b4 24 f0 49 00 00 4c 89 f7 e8 71 ec 1f c5 <0f> b6 5b 10 41 0f b6 84 24 30 4b 00 00 29 c3 41 0f b6 84 24 28 4b RSP: 0018:ffffc902b179fb60 EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: ffff8882c1398000 RSI: ffff888c7435cc60 RDI: ffff888c7435cb90 RBP: ffff888c7435cc60 R08: ffffc902b179fbb0 R09: 0000000000000000 R10: ffff888ef1fc8050 R11: fffffffffff82700 R12: ffff888c743581a0 R13: ffffc902b179fbb0 R14: ffff888c7435cb90 R15: 0000000000000000 FS: 00007fdc7dae0740(0000) GS:ffff888c105c0000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000010 CR3: 0000000132c24002 CR4: 00000000007706e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: <TASK> ? __die+0x20/0x70 ? page_fault_oops+0x76/0x170 ? exc_page_fault+0x65/0x150 ? asm_exc_page_fault+0x22/0x30 ? ice_dpll_rclk_state_on_pin_get+0x2f/0x90 [ice] ? __pfx_ice_dpll_rclk_state_on_pin_get+0x10/0x10 [ice] dpll_msg_add_pin_parents+0x142/0x1d0 dpll_pin_event_send+0x7d/0x150 dpll_pin_on_pin_unregister+0x3f/0x100 ice_dpll_deinit_pins+0xa1/0x230 [ice] ice_dpll_deinit+0x29/0xe0 [ice] ice_remove+0xcd/0x200 [ice] pci_device_remove+0x33/0xa0 device_release_driver_internal+0x193/0x200 unbind_store+0x9d/0xb0 kernfs_fop_write_iter+0x128/0x1c0 vfs_write+0x2bb/0x3e0 ksys_write+0x5f/0xe0 do_syscall_64+0x59/0x90 ? filp_close+0x1b/0x30 ? do_dup2+0x7d/0xd0 ? syscall_exit_work+0x103/0x130 ? syscall_exit_to_user_mode+0x22/0x40 ? do_syscall_64+0x69/0x90 ? syscall_exit_work+0x103/0x130 ? syscall_exit_to_user_mode+0x22/0x40 ? do_syscall_64+0x69/0x90 entry_SYSCALL_64_after_hwframe+0x6e/0xd8 RIP: 0033:0x7fdc7d93eb97 Code: 0b 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24 RSP: 002b:00007fff2aa91028 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 RAX: ffffffffffffffda RBX: 000000000000000d RCX: 00007fdc7d93eb97 RDX: 000000000000000d RSI: 00005644814ec9b0 RDI: 0000000000000001 RBP: 00005644814ec9b0 R08: 0000000000000000 R09: 00007fdc7d9b14e0 R10: 00007fdc7d9b13e0 R11: 0000000000000246 R12: 000000000000000d R13: 00007fdc7d9fb780 R14: 000000000000000d R15: 00007fdc7d9f69e0 </TASK> Modules linked in: uinput vfio_pci vfio_pci_core vfio_iommu_type1 vfio irqbypass ixgbevf snd_seq_dummy snd_hrtimer snd_seq snd_timer snd_seq_device snd soundcore overlay qrtr rfkill vfat fat xfs libcrc32c rpcrdma sunrpc rdma_ucm ib_srpt ib_isert iscsi_target_mod target_core_mod ib_iser libiscsi scsi_transport_iscsi rdma_cm iw_cm ib_cm intel_rapl_msr intel_rapl_common intel_uncore_frequency intel_uncore_frequency_common isst_if_common skx_edac nfit libnvdimm ipmi_ssif x86_pkg_temp_thermal intel_powerclamp coretemp irdma rapl intel_cstate ib_uverbs iTCO_wdt iTCO_vendor_support acpi_ipmi intel_uncore mei_me ipmi_si pcspkr i2c_i801 ib_core mei ipmi_devintf intel_pch_thermal ioatdma i2c_smbus ipmi_msghandler lpc_ich joydev acpi_power_meter acpi_pad ext4 mbcache jbd2 sd_mod t10_pi sg ast i2c_algo_bit drm_shmem_helper drm_kms_helper ice crct10dif_pclmul ixgbe crc32_pclmul drm crc32c_intel ahci i40e libahci ghash_clmulni_intel libata mdio dca gnss wmi fuse [last unloaded: iavf] CR2: 0000000000000010 v6: - fix memory corruption on error path in patch [v5 2/4] ==================== Acked-by: Vadim Fedorenko <vadim.fedorenko@linux.dev> Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents ef48521 + 7dc5b18 commit 94fa82b

File tree

3 files changed

+100
-29
lines changed

3 files changed

+100
-29
lines changed

‎drivers/dpll/dpll_core.c‎

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ static u32 dpll_pin_xa_id;
2929
WARN_ON_ONCE(!xa_get_mark(&dpll_device_xa, (d)->id, DPLL_REGISTERED))
3030
#define ASSERT_DPLL_NOT_REGISTERED(d) \
3131
WARN_ON_ONCE(xa_get_mark(&dpll_device_xa, (d)->id, DPLL_REGISTERED))
32-
#define ASSERT_PIN_REGISTERED(p) \
33-
WARN_ON_ONCE(!xa_get_mark(&dpll_pin_xa, (p)->id, DPLL_REGISTERED))
3432

3533
struct dpll_device_registration {
3634
struct list_head list;
@@ -425,6 +423,53 @@ void dpll_device_unregister(struct dpll_device *dpll,
425423
}
426424
EXPORT_SYMBOL_GPL(dpll_device_unregister);
427425

426+
static void dpll_pin_prop_free(struct dpll_pin_properties *prop)
427+
{
428+
kfree(prop->package_label);
429+
kfree(prop->panel_label);
430+
kfree(prop->board_label);
431+
kfree(prop->freq_supported);
432+
}
433+
434+
static int dpll_pin_prop_dup(const struct dpll_pin_properties *src,
435+
struct dpll_pin_properties *dst)
436+
{
437+
memcpy(dst, src, sizeof(*dst));
438+
if (src->freq_supported && src->freq_supported_num) {
439+
size_t freq_size = src->freq_supported_num *
440+
sizeof(*src->freq_supported);
441+
dst->freq_supported = kmemdup(src->freq_supported,
442+
freq_size, GFP_KERNEL);
443+
if (!src->freq_supported)
444+
return -ENOMEM;
445+
}
446+
if (src->board_label) {
447+
dst->board_label = kstrdup(src->board_label, GFP_KERNEL);
448+
if (!dst->board_label)
449+
goto err_board_label;
450+
}
451+
if (src->panel_label) {
452+
dst->panel_label = kstrdup(src->panel_label, GFP_KERNEL);
453+
if (!dst->panel_label)
454+
goto err_panel_label;
455+
}
456+
if (src->package_label) {
457+
dst->package_label = kstrdup(src->package_label, GFP_KERNEL);
458+
if (!dst->package_label)
459+
goto err_package_label;
460+
}
461+
462+
return 0;
463+
464+
err_package_label:
465+
kfree(dst->panel_label);
466+
err_panel_label:
467+
kfree(dst->board_label);
468+
err_board_label:
469+
kfree(dst->freq_supported);
470+
return -ENOMEM;
471+
}
472+
428473
static struct dpll_pin *
429474
dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
430475
const struct dpll_pin_properties *prop)
@@ -441,20 +486,24 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
441486
if (WARN_ON(prop->type < DPLL_PIN_TYPE_MUX ||
442487
prop->type > DPLL_PIN_TYPE_MAX)) {
443488
ret = -EINVAL;
444-
goto err;
489+
goto err_pin_prop;
445490
}
446-
pin->prop = prop;
491+
ret = dpll_pin_prop_dup(prop, &pin->prop);
492+
if (ret)
493+
goto err_pin_prop;
447494
refcount_set(&pin->refcount, 1);
448495
xa_init_flags(&pin->dpll_refs, XA_FLAGS_ALLOC);
449496
xa_init_flags(&pin->parent_refs, XA_FLAGS_ALLOC);
450497
ret = xa_alloc_cyclic(&dpll_pin_xa, &pin->id, pin, xa_limit_32b,
451498
&dpll_pin_xa_id, GFP_KERNEL);
452499
if (ret)
453-
goto err;
500+
goto err_xa_alloc;
454501
return pin;
455-
err:
502+
err_xa_alloc:
456503
xa_destroy(&pin->dpll_refs);
457504
xa_destroy(&pin->parent_refs);
505+
dpll_pin_prop_free(&pin->prop);
506+
err_pin_prop:
458507
kfree(pin);
459508
return ERR_PTR(ret);
460509
}
@@ -514,6 +563,7 @@ void dpll_pin_put(struct dpll_pin *pin)
514563
xa_destroy(&pin->dpll_refs);
515564
xa_destroy(&pin->parent_refs);
516565
xa_erase(&dpll_pin_xa, pin->id);
566+
dpll_pin_prop_free(&pin->prop);
517567
kfree(pin);
518568
}
519569
mutex_unlock(&dpll_lock);
@@ -564,8 +614,6 @@ dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
564614
WARN_ON(!ops->state_on_dpll_get) ||
565615
WARN_ON(!ops->direction_get))
566616
return -EINVAL;
567-
if (ASSERT_DPLL_REGISTERED(dpll))
568-
return -EINVAL;
569617

570618
mutex_lock(&dpll_lock);
571619
if (WARN_ON(!(dpll->module == pin->module &&
@@ -636,15 +684,13 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
636684
unsigned long i, stop;
637685
int ret;
638686

639-
if (WARN_ON(parent->prop->type != DPLL_PIN_TYPE_MUX))
687+
if (WARN_ON(parent->prop.type != DPLL_PIN_TYPE_MUX))
640688
return -EINVAL;
641689

642690
if (WARN_ON(!ops) ||
643691
WARN_ON(!ops->state_on_pin_get) ||
644692
WARN_ON(!ops->direction_get))
645693
return -EINVAL;
646-
if (ASSERT_PIN_REGISTERED(parent))
647-
return -EINVAL;
648694

649695
mutex_lock(&dpll_lock);
650696
ret = dpll_xa_ref_pin_add(&pin->parent_refs, parent, ops, priv);

‎drivers/dpll/dpll_core.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ struct dpll_device {
4444
* @module: module of creator
4545
* @dpll_refs: hold referencees to dplls pin was registered with
4646
* @parent_refs: hold references to parent pins pin was registered with
47-
* @prop: pointer to pin properties given by registerer
47+
* @prop: pin properties copied from the registerer
4848
* @rclk_dev_name: holds name of device when pin can recover clock from it
4949
* @refcount: refcount
5050
**/
@@ -55,7 +55,7 @@ struct dpll_pin {
5555
struct module *module;
5656
struct xarray dpll_refs;
5757
struct xarray parent_refs;
58-
const struct dpll_pin_properties *prop;
58+
struct dpll_pin_properties prop;
5959
refcount_t refcount;
6060
};
6161

‎drivers/dpll/dpll_netlink.c‎

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -303,17 +303,17 @@ dpll_msg_add_pin_freq(struct sk_buff *msg, struct dpll_pin *pin,
303303
if (nla_put_64bit(msg, DPLL_A_PIN_FREQUENCY, sizeof(freq), &freq,
304304
DPLL_A_PIN_PAD))
305305
return -EMSGSIZE;
306-
for (fs = 0; fs < pin->prop->freq_supported_num; fs++) {
306+
for (fs = 0; fs < pin->prop.freq_supported_num; fs++) {
307307
nest = nla_nest_start(msg, DPLL_A_PIN_FREQUENCY_SUPPORTED);
308308
if (!nest)
309309
return -EMSGSIZE;
310-
freq = pin->prop->freq_supported[fs].min;
310+
freq = pin->prop.freq_supported[fs].min;
311311
if (nla_put_64bit(msg, DPLL_A_PIN_FREQUENCY_MIN, sizeof(freq),
312312
&freq, DPLL_A_PIN_PAD)) {
313313
nla_nest_cancel(msg, nest);
314314
return -EMSGSIZE;
315315
}
316-
freq = pin->prop->freq_supported[fs].max;
316+
freq = pin->prop.freq_supported[fs].max;
317317
if (nla_put_64bit(msg, DPLL_A_PIN_FREQUENCY_MAX, sizeof(freq),
318318
&freq, DPLL_A_PIN_PAD)) {
319319
nla_nest_cancel(msg, nest);
@@ -329,9 +329,9 @@ static bool dpll_pin_is_freq_supported(struct dpll_pin *pin, u32 freq)
329329
{
330330
int fs;
331331

332-
for (fs = 0; fs < pin->prop->freq_supported_num; fs++)
333-
if (freq >= pin->prop->freq_supported[fs].min &&
334-
freq <= pin->prop->freq_supported[fs].max)
332+
for (fs = 0; fs < pin->prop.freq_supported_num; fs++)
333+
if (freq >= pin->prop.freq_supported[fs].min &&
334+
freq <= pin->prop.freq_supported[fs].max)
335335
return true;
336336
return false;
337337
}
@@ -421,7 +421,7 @@ static int
421421
dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
422422
struct netlink_ext_ack *extack)
423423
{
424-
const struct dpll_pin_properties *prop = pin->prop;
424+
const struct dpll_pin_properties *prop = &pin->prop;
425425
struct dpll_pin_ref *ref;
426426
int ret;
427427

@@ -553,6 +553,24 @@ __dpll_device_change_ntf(struct dpll_device *dpll)
553553
return dpll_device_event_send(DPLL_CMD_DEVICE_CHANGE_NTF, dpll);
554554
}
555555

556+
static bool dpll_pin_available(struct dpll_pin *pin)
557+
{
558+
struct dpll_pin_ref *par_ref;
559+
unsigned long i;
560+
561+
if (!xa_get_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED))
562+
return false;
563+
xa_for_each(&pin->parent_refs, i, par_ref)
564+
if (xa_get_mark(&dpll_pin_xa, par_ref->pin->id,
565+
DPLL_REGISTERED))
566+
return true;
567+
xa_for_each(&pin->dpll_refs, i, par_ref)
568+
if (xa_get_mark(&dpll_device_xa, par_ref->dpll->id,
569+
DPLL_REGISTERED))
570+
return true;
571+
return false;
572+
}
573+
556574
/**
557575
* dpll_device_change_ntf - notify that the dpll device has been changed
558576
* @dpll: registered dpll pointer
@@ -579,7 +597,7 @@ dpll_pin_event_send(enum dpll_cmd event, struct dpll_pin *pin)
579597
int ret = -ENOMEM;
580598
void *hdr;
581599

582-
if (WARN_ON(!xa_get_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED)))
600+
if (!dpll_pin_available(pin))
583601
return -ENODEV;
584602

585603
msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
@@ -717,7 +735,7 @@ dpll_pin_on_pin_state_set(struct dpll_pin *pin, u32 parent_idx,
717735
int ret;
718736

719737
if (!(DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE &
720-
pin->prop->capabilities)) {
738+
pin->prop.capabilities)) {
721739
NL_SET_ERR_MSG(extack, "state changing is not allowed");
722740
return -EOPNOTSUPP;
723741
}
@@ -753,7 +771,7 @@ dpll_pin_state_set(struct dpll_device *dpll, struct dpll_pin *pin,
753771
int ret;
754772

755773
if (!(DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE &
756-
pin->prop->capabilities)) {
774+
pin->prop.capabilities)) {
757775
NL_SET_ERR_MSG(extack, "state changing is not allowed");
758776
return -EOPNOTSUPP;
759777
}
@@ -780,7 +798,7 @@ dpll_pin_prio_set(struct dpll_device *dpll, struct dpll_pin *pin,
780798
int ret;
781799

782800
if (!(DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE &
783-
pin->prop->capabilities)) {
801+
pin->prop.capabilities)) {
784802
NL_SET_ERR_MSG(extack, "prio changing is not allowed");
785803
return -EOPNOTSUPP;
786804
}
@@ -808,7 +826,7 @@ dpll_pin_direction_set(struct dpll_pin *pin, struct dpll_device *dpll,
808826
int ret;
809827

810828
if (!(DPLL_PIN_CAPABILITIES_DIRECTION_CAN_CHANGE &
811-
pin->prop->capabilities)) {
829+
pin->prop.capabilities)) {
812830
NL_SET_ERR_MSG(extack, "direction changing is not allowed");
813831
return -EOPNOTSUPP;
814832
}
@@ -838,8 +856,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
838856
int ret;
839857

840858
phase_adj = nla_get_s32(phase_adj_attr);
841-
if (phase_adj > pin->prop->phase_range.max ||
842-
phase_adj < pin->prop->phase_range.min) {
859+
if (phase_adj > pin->prop.phase_range.max ||
860+
phase_adj < pin->prop.phase_range.min) {
843861
NL_SET_ERR_MSG_ATTR(extack, phase_adj_attr,
844862
"phase adjust value not supported");
845863
return -EINVAL;
@@ -1023,7 +1041,7 @@ dpll_pin_find(u64 clock_id, struct nlattr *mod_name_attr,
10231041
unsigned long i;
10241042

10251043
xa_for_each_marked(&dpll_pin_xa, i, pin, DPLL_REGISTERED) {
1026-
prop = pin->prop;
1044+
prop = &pin->prop;
10271045
cid_match = clock_id ? pin->clock_id == clock_id : true;
10281046
mod_match = mod_name_attr && module_name(pin->module) ?
10291047
!nla_strcmp(mod_name_attr,
@@ -1130,6 +1148,10 @@ int dpll_nl_pin_id_get_doit(struct sk_buff *skb, struct genl_info *info)
11301148
}
11311149
pin = dpll_pin_find_from_nlattr(info);
11321150
if (!IS_ERR(pin)) {
1151+
if (!dpll_pin_available(pin)) {
1152+
nlmsg_free(msg);
1153+
return -ENODEV;
1154+
}
11331155
ret = dpll_msg_add_pin_handle(msg, pin);
11341156
if (ret) {
11351157
nlmsg_free(msg);
@@ -1179,6 +1201,8 @@ int dpll_nl_pin_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
11791201

11801202
xa_for_each_marked_start(&dpll_pin_xa, i, pin, DPLL_REGISTERED,
11811203
ctx->idx) {
1204+
if (!dpll_pin_available(pin))
1205+
continue;
11821206
hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
11831207
cb->nlh->nlmsg_seq,
11841208
&dpll_nl_family, NLM_F_MULTI,
@@ -1441,7 +1465,8 @@ int dpll_pin_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
14411465
}
14421466
info->user_ptr[0] = xa_load(&dpll_pin_xa,
14431467
nla_get_u32(info->attrs[DPLL_A_PIN_ID]));
1444-
if (!info->user_ptr[0]) {
1468+
if (!info->user_ptr[0] ||
1469+
!dpll_pin_available(info->user_ptr[0])) {
14451470
NL_SET_ERR_MSG(info->extack, "pin not found");
14461471
ret = -ENODEV;
14471472
goto unlock_dev;

0 commit comments

Comments
 (0)