public-mirrors/linux
1
0
Fork 0
mirror of git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2025-08-05 16:54:27 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

pinmux: Use sequential access to access desc->pinmux data

Browse source 
When two client of the same gpio call pinctrl_select_state() for the
same functionality, we are seeing NULL pointer issue while accessing
desc->mux_owner.

Let's say two processes A, B executing in pin_request() for the same pin
and process A updates the desc->mux_usecount but not yet updated the
desc->mux_owner while process B see the desc->mux_usecount which got
updated by A path and further executes strcmp and while accessing
desc->mux_owner it crashes with NULL pointer.

Serialize the access to mux related setting with a mutex lock.

	cpu0 (process A)			cpu1(process B)

pinctrl_select_state() {		  pinctrl_select_state() {
  pin_request() {				pin_request() {
  ...
						 ....
    } else {
         desc->mux_usecount++;
    						desc->mux_usecount && strcmp(desc->mux_owner, owner)) {

         if (desc->mux_usecount > 1)
               return 0;
         desc->mux_owner = owner;

  }						}

Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
Link: https://lore.kernel.org/20241014192930.1539673-1-quic_mojha@quicinc.com
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
This commit is contained in:
Mukesh Ojha 2024-10-15 00:59:30 +05:30 • committed by Linus Walleij
parent 56c9d1a033
commit 5a3e85c3c3
3 changed files with 100 additions and 77 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
drivers/pinctrl/core.c
View file

@ -220,6 +220,9 @@ static int pinctrl_register_one_pin(struct pinctrl_dev *pctldev,
/* Set owner */
pindesc->pctldev = pctldev;
#ifdef CONFIG_PINMUX
mutex_init(&pindesc->mux_lock);
#endif
/* Copy basic pin info */
if (pin->name) {

1
drivers/pinctrl/core.h
View file

@ -177,6 +177,7 @@ struct pin_desc {
const char *mux_owner;
const struct pinctrl_setting_mux *mux_setting;
const char *gpio_owner;
struct mutex mux_lock;
#endif
};

173
drivers/pinctrl/pinmux.c
View file

@ -14,6 +14,7 @@
#include <linux/array_size.h>
#include <linux/ctype.h>
#include <linux/cleanup.h>
#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/err.h>
@ -93,6 +94,7 @@ bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev, unsigned int pin)
if (!desc || !ops)
return true;
guard(mutex)(&desc->mux_lock);
if (ops->strict && desc->mux_usecount)
return false;
@ -127,29 +129,31 @@ static int pin_request(struct pinctrl_dev *pctldev,
dev_dbg(pctldev->dev, "request pin %d (%s) for %s\n",
pin, desc->name, owner);
if ((!gpio_range || ops->strict) &&
desc->mux_usecount && strcmp(desc->mux_owner, owner)) {
dev_err(pctldev->dev,
"pin %s already requested by %s; cannot claim for %s\n",
desc->name, desc->mux_owner, owner);
goto out;
}
scoped_guard(mutex, &desc->mux_lock) {
if ((!gpio_range || ops->strict) &&
desc->mux_usecount && strcmp(desc->mux_owner, owner)) {
dev_err(pctldev->dev,
"pin %s already requested by %s; cannot claim for %s\n",
desc->name, desc->mux_owner, owner);
goto out;
}
if ((gpio_range || ops->strict) && desc->gpio_owner) {
dev_err(pctldev->dev,
"pin %s already requested by %s; cannot claim for %s\n",
desc->name, desc->gpio_owner, owner);
goto out;
}
if ((gpio_range || ops->strict) && desc->gpio_owner) {
dev_err(pctldev->dev,
"pin %s already requested by %s; cannot claim for %s\n",
desc->name, desc->gpio_owner, owner);
goto out;
}
if (gpio_range) {
desc->gpio_owner = owner;
} else {
desc->mux_usecount++;
if (desc->mux_usecount > 1)
return 0;
if (gpio_range) {
desc->gpio_owner = owner;
} else {
desc->mux_usecount++;
if (desc->mux_usecount > 1)
return 0;
desc->mux_owner = owner;
desc->mux_owner = owner;
}
}
/* Let each pin increase references to this module */
@ -178,12 +182,14 @@ static int pin_request(struct pinctrl_dev *pctldev,
out_free_pin:
if (status) {
if (gpio_range) {
desc->gpio_owner = NULL;
} else {
desc->mux_usecount--;
if (!desc->mux_usecount)
desc->mux_owner = NULL;
scoped_guard(mutex, &desc->mux_lock) {
if (gpio_range) {
desc->gpio_owner = NULL;
} else {
desc->mux_usecount--;
if (!desc->mux_usecount)
desc->mux_owner = NULL;
}
}
}
out:
@ -219,15 +225,17 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin,
return NULL;
}
if (!gpio_range) {
/*
* A pin should not be freed more times than allocated.
*/
if (WARN_ON(!desc->mux_usecount))
return NULL;
desc->mux_usecount--;
if (desc->mux_usecount)
return NULL;
scoped_guard(mutex, &desc->mux_lock) {
if (!gpio_range) {
/*
* A pin should not be freed more times than allocated.
*/
if (WARN_ON(!desc->mux_usecount))
return NULL;
desc->mux_usecount--;
if (desc->mux_usecount)
return NULL;
}
}
/*
@ -239,13 +247,15 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin,
else if (ops->free)
ops->free(pctldev, pin);
if (gpio_range) {
owner = desc->gpio_owner;
desc->gpio_owner = NULL;
} else {
owner = desc->mux_owner;
desc->mux_owner = NULL;
desc->mux_setting = NULL;
scoped_guard(mutex, &desc->mux_lock) {
if (gpio_range) {
owner = desc->gpio_owner;
desc->gpio_owner = NULL;
} else {
owner = desc->mux_owner;
desc->mux_owner = NULL;
desc->mux_setting = NULL;
}
}
module_put(pctldev->owner);
@ -458,7 +468,8 @@ int pinmux_enable_setting(const struct pinctrl_setting *setting)
pins[i]);
continue;
}
desc->mux_setting = &(setting->data.mux);
scoped_guard(mutex, &desc->mux_lock)
desc->mux_setting = &(setting->data.mux);
}
ret = ops->set_mux(pctldev, setting->data.mux.func,
@ -472,8 +483,10 @@ int pinmux_enable_setting(const struct pinctrl_setting *setting)
err_set_mux:
for (i = 0; i < num_pins; i++) {
desc = pin_desc_get(pctldev, pins[i]);
if (desc)
desc->mux_setting = NULL;
if (desc) {
scoped_guard(mutex, &desc->mux_lock)
desc->mux_setting = NULL;
}
}
err_pin_request:
/* On error release all taken pins */
@ -492,6 +505,7 @@ void pinmux_disable_setting(const struct pinctrl_setting *setting)
unsigned int num_pins = 0;
int i;
struct pin_desc *desc;
bool is_equal;
if (pctlops->get_group_pins)
ret = pctlops->get_group_pins(pctldev, setting->data.mux.group,
@ -517,7 +531,10 @@ void pinmux_disable_setting(const struct pinctrl_setting *setting)
pins[i]);
continue;
}
if (desc->mux_setting == &(setting->data.mux)) {
scoped_guard(mutex, &desc->mux_lock)
is_equal = (desc->mux_setting == &(setting->data.mux));
if (is_equal) {
pin_free(pctldev, pins[i], NULL);
} else {
const char *gname;
@ -608,40 +625,42 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
if (desc == NULL)
continue;
if (desc->mux_owner &&
!strcmp(desc->mux_owner, pinctrl_dev_get_name(pctldev)))
is_hog = true;
scoped_guard(mutex, &desc->mux_lock) {
if (desc->mux_owner &&
!strcmp(desc->mux_owner, pinctrl_dev_get_name(pctldev)))
is_hog = true;
if (pmxops->strict) {
if (desc->mux_owner)
seq_printf(s, "pin %d (%s): device %s%s",
pin, desc->name, desc->mux_owner,
if (pmxops->strict) {
if (desc->mux_owner)
seq_printf(s, "pin %d (%s): device %s%s",
pin, desc->name, desc->mux_owner,
is_hog ? " (HOG)" : "");
else if (desc->gpio_owner)
seq_printf(s, "pin %d (%s): GPIO %s",
pin, desc->name, desc->gpio_owner);
else
seq_printf(s, "pin %d (%s): UNCLAIMED",
pin, desc->name);
} else {
/* For non-strict controllers */
seq_printf(s, "pin %d (%s): %s %s%s", pin, desc->name,
desc->mux_owner ? desc->mux_owner
: "(MUX UNCLAIMED)",
desc->gpio_owner ? desc->gpio_owner
: "(GPIO UNCLAIMED)",
is_hog ? " (HOG)" : "");
else if (desc->gpio_owner)
seq_printf(s, "pin %d (%s): GPIO %s",
pin, desc->name, desc->gpio_owner);
else
seq_printf(s, "pin %d (%s): UNCLAIMED",
pin, desc->name);
} else {
/* For non-strict controllers */
seq_printf(s, "pin %d (%s): %s %s%s", pin, desc->name,
desc->mux_owner ? desc->mux_owner
: "(MUX UNCLAIMED)",
desc->gpio_owner ? desc->gpio_owner
: "(GPIO UNCLAIMED)",
is_hog ? " (HOG)" : "");
}
}
/* If mux: print function+group claiming the pin */
if (desc->mux_setting)
seq_printf(s, " function %s group %s\n",
pmxops->get_function_name(pctldev,
desc->mux_setting->func),
pctlops->get_group_name(pctldev,
desc->mux_setting->group));
else
seq_putc(s, '\n');
/* If mux: print function+group claiming the pin */
if (desc->mux_setting)
seq_printf(s, " function %s group %s\n",
pmxops->get_function_name(pctldev,
desc->mux_setting->func),
pctlops->get_group_name(pctldev,
desc->mux_setting->group));
else
seq_putc(s, '\n');
}
}
mutex_unlock(&pctldev->mutex);