Skip to content

Commit

Permalink
dpll: fix pin dump crash for rebound module
Browse files Browse the repository at this point in the history 
When a kernel module is unbound but the pin resources were not entirely
freed (other kernel module instance of the same PCI device have had kept
the reference to that pin), and kernel module is again bound, the pin
properties would not be updated (the properties are only assigned when
memory for the pin is allocated), prop pointer still points to the
kernel module memory of the kernel module which was deallocated on the
unbind.

If the pin dump is invoked in this state, the result is a kernel crash.
Prevent the crash by storing persistent pin properties in dpll subsystem,
copy the content from the kernel module when pin is allocated, instead of
using memory of the kernel module.

Fixes: 9431063 ("dpll: core: Add DPLL framework base functions")
Fixes: 9d71b54 ("dpll: netlink: Add DPLL framework base functions")
Reviewed-by: Jan Glaza <jan.glaza@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Arkadiusz Kubalewski authored and David S. Miller committed Jan 22, 2024
1 parent b6a11a7 commit 830ead5
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 18 deletions.
55 changes: 53 additions & 2 deletions drivers/dpll/dpll_core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,53 @@ void dpll_device_unregister(struct dpll_device *dpll,
}
EXPORT_SYMBOL_GPL(dpll_device_unregister);

static void dpll_pin_prop_free(struct dpll_pin_properties *prop)
{
kfree(prop->package_label);
kfree(prop->panel_label);
kfree(prop->board_label);
kfree(prop->freq_supported);
}

static int dpll_pin_prop_dup(const struct dpll_pin_properties *src,
struct dpll_pin_properties *dst)
{
memcpy(dst, src, sizeof(*dst));
if (src->freq_supported && src->freq_supported_num) {
size_t freq_size = src->freq_supported_num *
sizeof(*src->freq_supported);
dst->freq_supported = kmemdup(src->freq_supported,
freq_size, GFP_KERNEL);
if (!src->freq_supported)
return -ENOMEM;
}
if (src->board_label) {
dst->board_label = kstrdup(src->board_label, GFP_KERNEL);
if (!dst->board_label)
goto err_board_label;
}
if (src->panel_label) {
dst->panel_label = kstrdup(src->panel_label, GFP_KERNEL);
if (!dst->panel_label)
goto err_panel_label;
}
if (src->package_label) {
dst->package_label = kstrdup(src->package_label, GFP_KERNEL);
if (!dst->package_label)
goto err_package_label;
}

return 0;

err_package_label:
kfree(dst->panel_label);
err_panel_label:
kfree(dst->board_label);
err_board_label:
kfree(dst->freq_supported);
return -ENOMEM;
}

static struct dpll_pin *
dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
const struct dpll_pin_properties *prop)
Expand All @@ -443,7 +490,9 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
ret = -EINVAL;
goto err_pin_prop;
}
pin->prop = prop;
ret = dpll_pin_prop_dup(prop, &pin->prop);
if (ret)
goto err_pin_prop;
refcount_set(&pin->refcount, 1);
xa_init_flags(&pin->dpll_refs, XA_FLAGS_ALLOC);
xa_init_flags(&pin->parent_refs, XA_FLAGS_ALLOC);
Expand All @@ -455,6 +504,7 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
err_xa_alloc:
xa_destroy(&pin->dpll_refs);
xa_destroy(&pin->parent_refs);
dpll_pin_prop_free(&pin->prop);
err_pin_prop:
kfree(pin);
return ERR_PTR(ret);
Expand Down Expand Up @@ -515,6 +565,7 @@ void dpll_pin_put(struct dpll_pin *pin)
xa_destroy(&pin->dpll_refs);
xa_destroy(&pin->parent_refs);
xa_erase(&dpll_pin_xa, pin->id);
dpll_pin_prop_free(&pin->prop);
kfree(pin);
}
mutex_unlock(&dpll_lock);
Expand Down Expand Up @@ -637,7 +688,7 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
unsigned long i, stop;
int ret;

if (WARN_ON(parent->prop->type != DPLL_PIN_TYPE_MUX))
if (WARN_ON(parent->prop.type != DPLL_PIN_TYPE_MUX))
return -EINVAL;

if (WARN_ON(!ops) ||
Expand Down
4 changes: 2 additions & 2 deletions drivers/dpll/dpll_core.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ struct dpll_device {
* @module: module of creator
* @dpll_refs: hold referencees to dplls pin was registered with
* @parent_refs: hold references to parent pins pin was registered with
* @prop: pointer to pin properties given by registerer
* @prop: pin properties copied from the registerer
* @rclk_dev_name: holds name of device when pin can recover clock from it
* @refcount: refcount
**/
Expand All @@ -55,7 +55,7 @@ struct dpll_pin {
struct module *module;
struct xarray dpll_refs;
struct xarray parent_refs;
const struct dpll_pin_properties *prop;
struct dpll_pin_properties prop;
refcount_t refcount;
};

Expand Down
28 changes: 14 additions & 14 deletions drivers/dpll/dpll_netlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,17 +303,17 @@ dpll_msg_add_pin_freq(struct sk_buff *msg, struct dpll_pin *pin,
if (nla_put_64bit(msg, DPLL_A_PIN_FREQUENCY, sizeof(freq), &freq,
DPLL_A_PIN_PAD))
return -EMSGSIZE;
for (fs = 0; fs < pin->prop->freq_supported_num; fs++) {
for (fs = 0; fs < pin->prop.freq_supported_num; fs++) {
nest = nla_nest_start(msg, DPLL_A_PIN_FREQUENCY_SUPPORTED);
if (!nest)
return -EMSGSIZE;
freq = pin->prop->freq_supported[fs].min;
freq = pin->prop.freq_supported[fs].min;
if (nla_put_64bit(msg, DPLL_A_PIN_FREQUENCY_MIN, sizeof(freq),
&freq, DPLL_A_PIN_PAD)) {
nla_nest_cancel(msg, nest);
return -EMSGSIZE;
}
freq = pin->prop->freq_supported[fs].max;
freq = pin->prop.freq_supported[fs].max;
if (nla_put_64bit(msg, DPLL_A_PIN_FREQUENCY_MAX, sizeof(freq),
&freq, DPLL_A_PIN_PAD)) {
nla_nest_cancel(msg, nest);
Expand All @@ -329,9 +329,9 @@ static bool dpll_pin_is_freq_supported(struct dpll_pin *pin, u32 freq)
{
int fs;

for (fs = 0; fs < pin->prop->freq_supported_num; fs++)
if (freq >= pin->prop->freq_supported[fs].min &&
freq <= pin->prop->freq_supported[fs].max)
for (fs = 0; fs < pin->prop.freq_supported_num; fs++)
if (freq >= pin->prop.freq_supported[fs].min &&
freq <= pin->prop.freq_supported[fs].max)
return true;
return false;
}
Expand Down Expand Up @@ -421,7 +421,7 @@ static int
dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
struct netlink_ext_ack *extack)
{
const struct dpll_pin_properties *prop = pin->prop;
const struct dpll_pin_properties *prop = &pin->prop;
struct dpll_pin_ref *ref;
int ret;

Expand Down Expand Up @@ -717,7 +717,7 @@ dpll_pin_on_pin_state_set(struct dpll_pin *pin, u32 parent_idx,
int ret;

if (!(DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE &
pin->prop->capabilities)) {
pin->prop.capabilities)) {
NL_SET_ERR_MSG(extack, "state changing is not allowed");
return -EOPNOTSUPP;
}
Expand Down Expand Up @@ -753,7 +753,7 @@ dpll_pin_state_set(struct dpll_device *dpll, struct dpll_pin *pin,
int ret;

if (!(DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE &
pin->prop->capabilities)) {
pin->prop.capabilities)) {
NL_SET_ERR_MSG(extack, "state changing is not allowed");
return -EOPNOTSUPP;
}
Expand All @@ -780,7 +780,7 @@ dpll_pin_prio_set(struct dpll_device *dpll, struct dpll_pin *pin,
int ret;

if (!(DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE &
pin->prop->capabilities)) {
pin->prop.capabilities)) {
NL_SET_ERR_MSG(extack, "prio changing is not allowed");
return -EOPNOTSUPP;
}
Expand Down Expand Up @@ -808,7 +808,7 @@ dpll_pin_direction_set(struct dpll_pin *pin, struct dpll_device *dpll,
int ret;

if (!(DPLL_PIN_CAPABILITIES_DIRECTION_CAN_CHANGE &
pin->prop->capabilities)) {
pin->prop.capabilities)) {
NL_SET_ERR_MSG(extack, "direction changing is not allowed");
return -EOPNOTSUPP;
}
Expand Down Expand Up @@ -838,8 +838,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
int ret;

phase_adj = nla_get_s32(phase_adj_attr);
if (phase_adj > pin->prop->phase_range.max ||
phase_adj < pin->prop->phase_range.min) {
if (phase_adj > pin->prop.phase_range.max ||
phase_adj < pin->prop.phase_range.min) {
NL_SET_ERR_MSG_ATTR(extack, phase_adj_attr,
"phase adjust value not supported");
return -EINVAL;
Expand Down Expand Up @@ -1023,7 +1023,7 @@ dpll_pin_find(u64 clock_id, struct nlattr *mod_name_attr,
unsigned long i;

xa_for_each_marked(&dpll_pin_xa, i, pin, DPLL_REGISTERED) {
prop = pin->prop;
prop = &pin->prop;
cid_match = clock_id ? pin->clock_id == clock_id : true;
mod_match = mod_name_attr && module_name(pin->module) ?
!nla_strcmp(mod_name_attr,
Expand Down

0 comments on commit 830ead5

Please sign in to comment.