linux-kernel - Re: [PATCH] pinctrl: Don't just pretend to protect pinctrl

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACRpkdbb320-fKbrEb075gDhD=A_16huZaO06KDmL-1F9Xx+dQ@mail.gmail.com>
Date:	Wed, 6 May 2015 16:25:19 +0200
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Doug Anderson <dianders@...omium.org>
Cc:	Patrice Chotard <patrice.chotard@...com>,
	Stephen Warren <swarren@...dotorg.org>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Heiko Stuebner <heiko@...ech.de>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] pinctrl: Don't just pretend to protect pinctrl_maps, do
 it for real

On Fri, May 1, 2015 at 6:01 PM, Doug Anderson <dianders@...omium.org> wrote:

> Way back, when the world was a simpler place and there was no war, no
> evil, and no kernel bugs, there was just a single pinctrl lock.  That
> was how the world was when (57291ce pinctrl: core device tree mapping
> table parsing support) was written.  In that case, there were
> instances where the pinctrl mutex was already held when
> pinctrl_register_map() was called, hence a "locked" parameter was
> passed to the function to indicate that the mutex was already locked
> (so we shouldn't lock it again).
>
> A few years ago in (42fed7b pinctrl: move subsystem mutex to
> pinctrl_dev struct), we switched to a separate pinctrl_maps_mutex.
> ...but (oops) we forgot to re-think about the whole "locked" parameter
> for pinctrl_register_map().  Basically the "locked" parameter appears
> to still refer to whether the bigger pinctrl_dev mutex is locked, but
> we're using it to skip locks of our (now separate) pinctrl_maps_mutex.
>
> That's kind of a bad thing(TM).  Probably nobody noticed because most
> of the calls to pinctrl_register_map happen at boot time and we've got
> synchronous device probing.  ...and even cases where we're
> asynchronous don't end up actually hitting the race too often.  ...but
> after banging my head against the wall for a bug that reproduced 1 out
> of 1000 reboots and lots of looking through kgdb, I finally noticed
> this.
>
> Anyway, we can now safely remove the "locked" parameter and go back to
> a war-free, evil-free, and kernel-bug-free world.
>
> Fixes: 42fed7ba44e4 ("pinctrl: move subsystem mutex to pinctrl_dev struct")
> Signed-off-by: Doug Anderson <dianders@...omium.org>

GOOD CATCH.

Patch applied for fixes.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/