pinctrl_generic_add_group() documents that the caller must take care of
locking, and pinmux_generic_add_function() needs it too, but
pinctrl_generic_dt_node_to_map() calls them without holding
pctldev->mutex, and the core caller in create_pinctrl() does not take it
either.

The driver core calls pinctrl_bind_pins() before probing a device, so
two devices that reference the same pin controller can run
pinctrl_generic_dt_node_to_map() on one pctldev at the same time.

Both `add` functions take the new selector from pctldev->num_groups or
pctldev->num_functions, and radix_tree_insert() at that index.
Two racing callers can read the same selector before either
has inserted, so the second insert collides and fails:

  k1-pinctrl d401e000.pinctrl: 
  error -EEXIST: error adding function pcie2-0-cfg
  k1-pinctrl d401e000.pinctrl: 
  does not have pin group pcie0-0-cfg.pcie0-0-pins

leaving one consumer without its pin configuration.

This was hit on a SpacemiT K3 board, where PCIe devices probe in parallel
against the single shared pin controller.

Take pctldev->mutex across the whole function, so that the groups and the
function referring are in a single critical section.

Fixes: 43722575e5cd ("pinctrl: add generic functions + pins mapper")
Signed-off-by: Sarah Emery <sarah.emery@canonical.com>
---
 Notes:
   The other two users of this function, ultrarisc/pinctrl-ultrarisc.c and
   microchip/pinctrl-mpfs-mssio.c, are exposed to the same issue if two
   consumers of one controller probe concurrently. I don't have the hardware
   to check if I would hit the same issue.
 
   My reproducer needs SpacemiT K3 PCIe support, which is not
   upstream yet, only in linux-riscv mailing list. Runtime testing was on a
   v7.2-based tree with SpacemiT patches. Build succeeds on mainline.
 
   The two added includes are include-what-you-use, guard() and the
   mutex API already resolve through core.h.

 
 drivers/pinctrl/pinctrl-generic.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
index fd6bdb74028a..4277c8748513 100644
--- a/drivers/pinctrl/pinctrl-generic.c
+++ b/drivers/pinctrl/pinctrl-generic.c
@@ -3,8 +3,10 @@
 #define pr_fmt(fmt) "generic pinconfig core: " fmt
 
 #include <linux/array_size.h>
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/slab.h>
 
@@ -196,6 +198,8 @@ static int pinctrl_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
 	int ngroups = 0;
 	int ret;
 
+	guard(mutex)(&pctldev->mutex);
+
 	*maps = NULL;
 	*num_maps = 0;
 
-- 
2.53.0

On Fri, Aug 28, 2026 at 5:58 PM Sarah Emery <sarah.emery@canonical.com> wrote:

> pinctrl_generic_add_group() documents that the caller must take care of
> locking, and pinmux_generic_add_function() needs it too, but
> pinctrl_generic_dt_node_to_map() calls them without holding
> pctldev->mutex, and the core caller in create_pinctrl() does not take it
> either.
>
> The driver core calls pinctrl_bind_pins() before probing a device, so
> two devices that reference the same pin controller can run
> pinctrl_generic_dt_node_to_map() on one pctldev at the same time.
>
> Both `add` functions take the new selector from pctldev->num_groups or
> pctldev->num_functions, and radix_tree_insert() at that index.
> Two racing callers can read the same selector before either
> has inserted, so the second insert collides and fails:
>
>   k1-pinctrl d401e000.pinctrl:
>   error -EEXIST: error adding function pcie2-0-cfg
>   k1-pinctrl d401e000.pinctrl:
>   does not have pin group pcie0-0-cfg.pcie0-0-pins
>
> leaving one consumer without its pin configuration.
>
> This was hit on a SpacemiT K3 board, where PCIe devices probe in parallel
> against the single shared pin controller.
>
> Take pctldev->mutex across the whole function, so that the groups and the
> function referring are in a single critical section.
>
> Fixes: 43722575e5cd ("pinctrl: add generic functions + pins mapper")
> Signed-off-by: Sarah Emery <sarah.emery@canonical.com>

Patch applied for fixes, sorry for taking so long to respond.

Yours,
Linus Walleij