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 */ /* Set owner */
pindesc->pctldev = pctldev; pindesc->pctldev = pctldev;
#ifdef CONFIG_PINMUX
mutex_init(&pindesc->mux_lock);
#endif
/* Copy basic pin info */ /* Copy basic pin info */
if (pin->name) { if (pin->name) {

1
drivers/pinctrl/core.h
View file

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

173
drivers/pinctrl/pinmux.c
View file

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