[PATCH] pinctrl: generic: serialise pinctrl_generic_dt_node_to_map()

Sarah Emery <[email protected]>
Newsgroups org.kernel.vger.linux-gpio,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.