linux-kernel - Re: [PATCH v2] pinmux: fix race causing mux_owner NULL with active mux

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: <CACRpkdY-4qHvYYDC39Z5K1mUkeMvpy0b4S-az673OdqxE6uPtQ@mail.gmail.com>
Date: Fri, 11 Jul 2025 20:17:01 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: Mukesh Ojha <mukesh.ojha@....qualcomm.com>
Cc: Mukesh Ojha <quic_mojha@...cinc.com>, linux-gpio@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] pinmux: fix race causing mux_owner NULL with active mux_usecount

On Tue, Jul 8, 2025 at 9:59 AM Mukesh Ojha <mukesh.ojha@....qualcomm.com> wrote:

> commit 5a3e85c3c397 ("pinmux: Use sequential access to access
> desc->pinmux data") tried to address the issue when two client of the
> same gpio calls pinctrl_select_state() for the same functionality, was
> resulting in NULL pointer issue while accessing desc->mux_owner.
> However, issue was not completely fixed due to the way it was handled
> and it can still result in the same NULL pointer.
>
> The issue occurs due to the following interleaving:
>
>      cpu0 (process A)                   cpu1 (process B)
>
>       pin_request() {                   pin_free() {
>
>                                          mutex_lock()
>                                          desc->mux_usecount--; //becomes 0
>                                          ..
>                                          mutex_unlock()
>
>   mutex_lock(desc->mux)
>   desc->mux_usecount++; // becomes 1
>   desc->mux_owner = owner;
>   mutex_unlock(desc->mux)
>
>                                          mutex_lock(desc->mux)
>                                          desc->mux_owner = NULL;
>                                          mutex_unlock(desc->mux)
>
> This sequence leads to a state where the pin appears to be in use
> (`mux_usecount == 1`) but has no owner (`mux_owner == NULL`), which can
> cause NULL pointer on next pin_request on the same pin.
>
> Ensure that updates to mux_usecount and mux_owner are performed
> atomically under the same lock. Only clear mux_owner when mux_usecount
> reaches zero and no new owner has been assigned.
>
> Fixes: 5a3e85c3c397 ("pinmux: Use sequential access to access desc->pinmux data")
> Signed-off-by: Mukesh Ojha <mukesh.ojha@....qualcomm.com>

Thanks Mukesh, patch applied!

Yours,
Linus Walleij