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

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>

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
+	mutex_init(&pindesc->mux_lock);
+#endif
 
 	/* Copy basic pin info */
 	if (pin->name) {

drivers/pinctrl/core.h

@@ -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
 };
 

drivers/pinctrl/pinmux.c

@@ -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,6 +129,7 @@ static int pin_request(struct pinctrl_dev *pctldev,
 	dev_dbg(pctldev->dev, "request pin %d (%s) for %s\n",
 		pin, desc->name, owner);
 
+	scoped_guard(mutex, &desc->mux_lock) {
 		if ((!gpio_range || ops->strict) &&
 		    desc->mux_usecount && strcmp(desc->mux_owner, owner)) {
 			dev_err(pctldev->dev,
@@ -151,6 +154,7 @@ static int pin_request(struct pinctrl_dev *pctldev,
 
 			desc->mux_owner = owner;
 		}
+	}
 
 	/* Let each pin increase references to this module */
 	if (!try_module_get(pctldev->owner)) {
@@ -178,6 +182,7 @@ static int pin_request(struct pinctrl_dev *pctldev,
 
 out_free_pin:
 	if (status) {
+		scoped_guard(mutex, &desc->mux_lock) {
 			if (gpio_range) {
 				desc->gpio_owner = NULL;
 			} else {
@@ -186,6 +191,7 @@ out_free_pin:
 					desc->mux_owner = NULL;
 			}
 		}
+	}
 out:
 	if (status)
 		dev_err_probe(pctldev->dev, status, "pin-%d (%s)\n",
@@ -219,6 +225,7 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin,
 		return NULL;
 	}
 
+	scoped_guard(mutex, &desc->mux_lock) {
 		if (!gpio_range) {
 			/*
 			 * A pin should not be freed more times than allocated.
@@ -229,6 +236,7 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin,
 			if (desc->mux_usecount)
 				return NULL;
 		}
+	}
 
 	/*
 	 * If there is no kind of request function for the pin we just assume
@@ -239,6 +247,7 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin,
 	else if (ops->free)
 		ops->free(pctldev, pin);
 
+	scoped_guard(mutex, &desc->mux_lock) {
 		if (gpio_range) {
 			owner = desc->gpio_owner;
 			desc->gpio_owner = NULL;
@@ -247,6 +256,7 @@ static const char *pin_free(struct pinctrl_dev *pctldev, int pin,
 			desc->mux_owner = NULL;
 			desc->mux_setting = NULL;
 		}
+	}
 
 	module_put(pctldev->owner);
 
@@ -458,6 +468,7 @@ int pinmux_enable_setting(const struct pinctrl_setting *setting)
 				 pins[i]);
 			continue;
 		}
+		scoped_guard(mutex, &desc->mux_lock)
 			desc->mux_setting = &(setting->data.mux);
 	}
 
@@ -472,9 +483,11 @@ 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)
+		if (desc) {
+			scoped_guard(mutex, &desc->mux_lock)
 				desc->mux_setting = NULL;
 		}
+	}
 err_pin_request:
 	/* On error release all taken pins */
 	while (--i >= 0)
@@ -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,6 +625,7 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
 		if (desc == NULL)
 			continue;
 
+		scoped_guard(mutex, &desc->mux_lock) {
 			if (desc->mux_owner &&
 			    !strcmp(desc->mux_owner, pinctrl_dev_get_name(pctldev)))
 				is_hog = true;
@@ -643,6 +661,7 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
 			else
 				seq_putc(s, '\n');
 		}
+	}
 
 	mutex_unlock(&pctldev->mutex);