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. 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@xxxxxxxxxxx> --- drivers/pinctrl/core.c | 3 +++ drivers/pinctrl/core.h | 2 ++ drivers/pinctrl/pinmux.c | 21 +++++++++++++++++++-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c index 314ab93d7691..e451af82eff2 100644 --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -220,6 +220,9 @@ static int pinctrl_register_one_pin(struct pinctrl_dev *pctldev, /* Set owner */ pindesc->pctldev = pctldev; +#ifdef CONFIG_PINMUX + spin_lock_init(&pindesc->lock); +#endif /* Copy basic pin info */ if (pin->name) { diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h index 4e07707d2435..afc651ce3985 100644 --- a/drivers/pinctrl/core.h +++ b/drivers/pinctrl/core.h @@ -12,6 +12,7 @@ #include <linux/list.h> #include <linux/mutex.h> #include <linux/radix-tree.h> +#include <linux/spinlock.h> #include <linux/types.h> #include <linux/pinctrl/machine.h> @@ -177,6 +178,7 @@ struct pin_desc { const char *mux_owner; const struct pinctrl_setting_mux *mux_setting; const char *gpio_owner; + spinlock_t lock; #endif }; diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c index aae71a37219b..75735618646d 100644 --- a/drivers/pinctrl/pinmux.c +++ b/drivers/pinctrl/pinmux.c @@ -115,6 +115,7 @@ static int pin_request(struct pinctrl_dev *pctldev, struct pin_desc *desc; const struct pinmux_ops *ops = pctldev->desc->pmxops; int status = -EINVAL; + unsigned long flags; desc = pin_desc_get(pctldev, pin); if (desc == NULL) { @@ -127,6 +128,7 @@ static int pin_request(struct pinctrl_dev *pctldev, dev_dbg(pctldev->dev, "request pin %d (%s) for %s\n", pin, desc->name, owner); + spin_lock_irqsave(&desc->lock, flags); if ((!gpio_range || ops->strict) && desc->mux_usecount && strcmp(desc->mux_owner, owner)) { dev_err(pctldev->dev, @@ -151,6 +153,7 @@ static int pin_request(struct pinctrl_dev *pctldev, desc->mux_owner = owner; } + spin_unlock_irqrestore(&desc->lock, flags); /* Let each pin increase references to this module */ if (!try_module_get(pctldev->owner)) { @@ -178,6 +181,7 @@ static int pin_request(struct pinctrl_dev *pctldev, out_free_pin: if (status) { + spin_lock_irqsave(&desc->lock, flags); if (gpio_range) { desc->gpio_owner = NULL; } else { @@ -185,6 +189,7 @@ static int pin_request(struct pinctrl_dev *pctldev, if (!desc->mux_usecount) desc->mux_owner = NULL; } + spin_unlock_irqrestore(&desc->lock, flags); } out: if (status) @@ -211,6 +216,7 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin, const struct pinmux_ops *ops = pctldev->desc->pmxops; struct pin_desc *desc; const char *owner; + unsigned long flags; desc = pin_desc_get(pctldev, pin); if (desc == NULL) { @@ -223,11 +229,13 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin, /* * A pin should not be freed more times than allocated. */ + spin_lock_irqsave(&desc->lock, flags); if (WARN_ON(!desc->mux_usecount)) - return NULL; + goto out; desc->mux_usecount--; if (desc->mux_usecount) - return NULL; + goto out; + spin_unlock_irqrestore(&desc->lock, flags); } /* @@ -239,6 +247,7 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin, else if (ops->free) ops->free(pctldev, pin); + spin_lock_irqsave(&desc->lock, flags); if (gpio_range) { owner = desc->gpio_owner; desc->gpio_owner = NULL; @@ -247,10 +256,14 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin, desc->mux_owner = NULL; desc->mux_setting = NULL; } + spin_unlock_irqrestore(&desc->lock, flags); module_put(pctldev->owner); return owner; +out: + spin_unlock_irqrestore(&desc->lock, flags); + return NULL; } /** @@ -586,6 +599,7 @@ static int pinmux_pins_show(struct seq_file *s, void *what) const struct pinctrl_ops *pctlops = pctldev->desc->pctlops; const struct pinmux_ops *pmxops = pctldev->desc->pmxops; unsigned int i, pin; + unsigned long flags; if (!pmxops) return 0; @@ -611,6 +625,7 @@ static int pinmux_pins_show(struct seq_file *s, void *what) if (desc == NULL) continue; + spin_lock_irqsave(&desc->lock, flags); if (desc->mux_owner && !strcmp(desc->mux_owner, pinctrl_dev_get_name(pctldev))) is_hog = true; @@ -645,6 +660,8 @@ static int pinmux_pins_show(struct seq_file *s, void *what) desc->mux_setting->group)); else seq_putc(s, '\n'); + + spin_unlock_irqrestore(&desc->lock, flags); } mutex_unlock(&pctldev->mutex); -- 2.34.1