[PATCH] pinctrl: Add lock to ensure the state atomization

From: Maria Yu
Date: Fri Dec 01 2023 - 10:29:58 EST


Currently pinctrl_select_state is an export symbol and don't have
effective re-entrance protect design. And possible of pinctrl state
changed during pinctrl_commit_state handling. Add per pinctrl lock to
ensure the old state and new state transition atomization.
Move dev error print message right before old_state pinctrl_select_state
and out of lock protection to avoid console related driver call
pinctrl_select_state recursively.

Fixes: 4198a9b57106 ("pinctrl: avoid reload of p state in list iteration")
Signed-off-by: Maria Yu <quic_aiquny@xxxxxxxxxxx>
---
drivers/pinctrl/core.c | 11 +++++++++--
drivers/pinctrl/core.h | 2 ++
2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index f2977eb65522..a19c286bf82e 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1066,6 +1066,7 @@ static struct pinctrl *create_pinctrl(struct device *dev,
p->dev = dev;
INIT_LIST_HEAD(&p->states);
INIT_LIST_HEAD(&p->dt_maps);
+ spin_lock_init(&p->lock);

ret = pinctrl_dt_to_map(p, pctldev);
if (ret < 0) {
@@ -1262,9 +1263,12 @@ static void pinctrl_link_add(struct pinctrl_dev *pctldev,
static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state)
{
struct pinctrl_setting *setting, *setting2;
- struct pinctrl_state *old_state = READ_ONCE(p->state);
+ struct pinctrl_state *old_state;
int ret;
+ unsigned long flags;

+ spin_lock_irqsave(&p->lock, flags);
+ old_state = p->state;
if (old_state) {
/*
* For each pinmux setting in the old state, forget SW's record
@@ -1329,11 +1333,11 @@ static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state)
}

p->state = state;
+ spin_unlock_irqrestore(&p->lock, flags);

return 0;

unapply_new_state:
- dev_err(p->dev, "Error applying setting, reverse things back\n");

list_for_each_entry(setting2, &state->settings, node) {
if (&setting2->node == &setting->node)
@@ -1349,6 +1353,9 @@ static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state)
pinmux_disable_setting(setting2);
}

+ spin_unlock_irqrestore(&p->lock, flags);
+
+ dev_err(p->dev, "Error applying setting, reverse things back\n");
/* There's no infinite recursive loop here because p->state is NULL */
if (old_state)
pinctrl_select_state(p, old_state);
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 530370443c19..86fc41393f7b 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>
@@ -91,6 +92,7 @@ struct pinctrl {
struct pinctrl_state *state;
struct list_head dt_maps;
struct kref users;
+ spinlock_t lock;
};

/**

base-commit: 994d5c58e50e91bb02c7be4a91d5186292a895c8
--
2.17.1