* [PATCH] pinctrl: protect pinctrl_list add
@ 2014-02-03 11:39 Stanislaw Gruszka
2014-02-03 21:12 ` Stephen Warren
0 siblings, 1 reply; 6+ messages in thread
From: Stanislaw Gruszka @ 2014-02-03 11:39 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-kernel, Stephen Warren
We have few fedora bug reports about list corruption on pinctrl,
for example:
https://bugzilla.redhat.com/show_bug.cgi?id=1051918
Most likely corruption happen due lack of protection of pinctrl_list
when adding new nodes to it. Patch corrects that.
Fixes: 57b676f9c1b ("pinctrl: fix and simplify locking")
Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
I only compile tested patch, also did not get test results from bug
reporters yet. But I'm pretty confident that patch fixes the problem
and does not cause any other deadlock.
drivers/pinctrl/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 5ee61a4..cab020a 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -851,7 +851,9 @@ static struct pinctrl *create_pinctrl(struct device *dev)
kref_init(&p->users);
/* Add the pinctrl handle to the global list */
+ mutex_lock(&pinctrl_list_mutex);
list_add_tail(&p->node, &pinctrl_list);
+ mutex_unlock(&pinctrl_list_mutex);
return p;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] pinctrl: protect pinctrl_list add
2014-02-03 11:39 [PATCH] pinctrl: protect pinctrl_list add Stanislaw Gruszka
@ 2014-02-03 21:12 ` Stephen Warren
2014-02-04 8:01 ` Stanislaw Gruszka
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2014-02-03 21:12 UTC (permalink / raw)
To: Stanislaw Gruszka, Linus Walleij; +Cc: linux-kernel, Stephen Warren
On 02/03/2014 04:39 AM, Stanislaw Gruszka wrote:
> We have few fedora bug reports about list corruption on pinctrl,
> for example:
> https://bugzilla.redhat.com/show_bug.cgi?id=1051918
>
> Most likely corruption happen due lack of protection of pinctrl_list
> when adding new nodes to it. Patch corrects that.
>
> Fixes: 57b676f9c1b ("pinctrl: fix and simplify locking")
After that patch ...
> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
> @@ -851,7 +851,9 @@ static struct pinctrl *create_pinctrl(struct device *dev)
> kref_init(&p->users);
>
> /* Add the pinctrl handle to the global list */
> + mutex_lock(&pinctrl_list_mutex);
That variable doesn't exist; it got replaced with the "global"
pinctrl_mutex. Also, since that patch, IIRC some other changes have been
made to the locking structure, so this patch might need adjustments not
to conflict with those changes?
> list_add_tail(&p->node, &pinctrl_list);
> + mutex_unlock(&pinctrl_list_mutex);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pinctrl: protect pinctrl_list add
2014-02-03 21:12 ` Stephen Warren
@ 2014-02-04 8:01 ` Stanislaw Gruszka
2014-02-04 8:07 ` [PATCH v2] " Stanislaw Gruszka
0 siblings, 1 reply; 6+ messages in thread
From: Stanislaw Gruszka @ 2014-02-04 8:01 UTC (permalink / raw)
To: Stephen Warren; +Cc: Linus Walleij, linux-kernel, Stephen Warren
On Mon, Feb 03, 2014 at 02:12:51PM -0700, Stephen Warren wrote:
> On 02/03/2014 04:39 AM, Stanislaw Gruszka wrote:
> > We have few fedora bug reports about list corruption on pinctrl,
> > for example:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1051918
> >
> > Most likely corruption happen due lack of protection of pinctrl_list
> > when adding new nodes to it. Patch corrects that.
> >
> > Fixes: 57b676f9c1b ("pinctrl: fix and simplify locking")
>
> After that patch ...
>
> > diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
>
> > @@ -851,7 +851,9 @@ static struct pinctrl *create_pinctrl(struct device *dev)
> > kref_init(&p->users);
> >
> > /* Add the pinctrl handle to the global list */
> > + mutex_lock(&pinctrl_list_mutex);
>
> That variable doesn't exist; it got replaced with the "global"
> pinctrl_mutex. Also, since that patch, IIRC some other changes have been
> made to the locking structure, so this patch might need adjustments not
> to conflict with those changes?
I missed that, I just quicky looked at git blame. This mutex was added
again by commit 42fed7ba44e4e8c1fb27b28ad14490cb1daff3c7
"pinctrl: move subsystem mutex to pinctrl_dev struct"
and actually this is the commit that introduce the bug. Before it,
list_add_tail() was called inside pinctrl_get_locked() and was
protected by global pinctrl_mutex.
I'll post patch with fixed "Fixes:" shortly.
Thanks
Stanislaw
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] pinctrl: protect pinctrl_list add
2014-02-04 8:01 ` Stanislaw Gruszka
@ 2014-02-04 8:07 ` Stanislaw Gruszka
2014-02-04 17:09 ` Stephen Warren
2014-02-04 21:08 ` Linus Walleij
0 siblings, 2 replies; 6+ messages in thread
From: Stanislaw Gruszka @ 2014-02-04 8:07 UTC (permalink / raw)
To: Stephen Warren, Linus Walleij
Cc: linux-kernel, Stephen Warren, Patrice Chotard
We have few fedora bug reports about list corruption on pinctrl,
for example:
https://bugzilla.redhat.com/show_bug.cgi?id=1051918
Most likely corruption happen due lack of protection of pinctrl_list
when adding new nodes to it. Patch corrects that.
Fixes: 42fed7ba44e ("pinctrl: move subsystem mutex to pinctrl_dev struct")
Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
I only compile tested patch, also did not get test results from bug
reporters yet. But I'm pretty confident that patch fixes the problem
and does not cause any other deadlock.
v1 -> v2: point correct first bad commit.
drivers/pinctrl/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 5ee61a4..cab020a 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -851,7 +851,9 @@ static struct pinctrl *create_pinctrl(struct device *dev)
kref_init(&p->users);
/* Add the pinctrl handle to the global list */
+ mutex_lock(&pinctrl_list_mutex);
list_add_tail(&p->node, &pinctrl_list);
+ mutex_unlock(&pinctrl_list_mutex);
return p;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] pinctrl: protect pinctrl_list add
2014-02-04 8:07 ` [PATCH v2] " Stanislaw Gruszka
@ 2014-02-04 17:09 ` Stephen Warren
2014-02-04 21:08 ` Linus Walleij
1 sibling, 0 replies; 6+ messages in thread
From: Stephen Warren @ 2014-02-04 17:09 UTC (permalink / raw)
To: Stanislaw Gruszka, Linus Walleij
Cc: linux-kernel, Stephen Warren, Patrice Chotard
On 02/04/2014 01:07 AM, Stanislaw Gruszka wrote:
> We have few fedora bug reports about list corruption on pinctrl,
> for example:
> https://bugzilla.redhat.com/show_bug.cgi?id=1051918
>
> Most likely corruption happen due lack of protection of pinctrl_list
> when adding new nodes to it. Patch corrects that.
>
> Fixes: 42fed7ba44e ("pinctrl: move subsystem mutex to pinctrl_dev struct")
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
(but not tested)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] pinctrl: protect pinctrl_list add
2014-02-04 8:07 ` [PATCH v2] " Stanislaw Gruszka
2014-02-04 17:09 ` Stephen Warren
@ 2014-02-04 21:08 ` Linus Walleij
1 sibling, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2014-02-04 21:08 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Stephen Warren, linux-kernel, Stephen Warren, Patrice Chotard
On Tue, Feb 4, 2014 at 9:07 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> We have few fedora bug reports about list corruption on pinctrl,
> for example:
> https://bugzilla.redhat.com/show_bug.cgi?id=1051918
>
> Most likely corruption happen due lack of protection of pinctrl_list
> when adding new nodes to it. Patch corrects that.
>
> Fixes: 42fed7ba44e ("pinctrl: move subsystem mutex to pinctrl_dev struct")
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
> I only compile tested patch, also did not get test results from bug
> reporters yet. But I'm pretty confident that patch fixes the problem
> and does not cause any other deadlock.
>
> v1 -> v2: point correct first bad commit.
Patch applied. Thanks for bringing this to our attention!
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-04 22:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-03 11:39 [PATCH] pinctrl: protect pinctrl_list add Stanislaw Gruszka
2014-02-03 21:12 ` Stephen Warren
2014-02-04 8:01 ` Stanislaw Gruszka
2014-02-04 8:07 ` [PATCH v2] " Stanislaw Gruszka
2014-02-04 17:09 ` Stephen Warren
2014-02-04 21:08 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).