| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240912142447.981590-1-quic_mojha@quicinc.com>
Date: Thu, 12 Sep 2024 19:54:47 +0530
From: Mukesh Ojha <quic_mojha@...cinc.com>
To: Linus Walleij <linus.walleij@...aro.org>
CC: <linux-gpio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
Mukesh Ojha
<quic_mojha@...cinc.com>
Subject: [RFC PATCH] 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.
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@...cinc.com>
---
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
Powered by blists - more mailing lists