linux-kernel - [rfc 1/2] pinctrl: pinconf-generic: perform basic checks on pincfg properties

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260203-herald-wrench-990ca2aaed2a@spud>
Date: Tue,  3 Feb 2026 17:29:42 +0000
From: Conor Dooley <conor@...nel.org>
To: linusw@...nel.org
Cc: conor@...nel.org,
	Conor Dooley <conor.dooley@...rochip.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	linux-gpio@...r.kernel.org,
	devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [rfc 1/2] pinctrl: pinconf-generic: perform basic checks on pincfg properties

From: Conor Dooley <conor.dooley@...rochip.com>

Some pinconf properties are mutually exclusive, either because they
convey the same information in different units or represent incompatible
configurations of the same pin. Attempt, in two ways, to prevent these
situations.

Firstly, for enable/disable properties, produce an error if both are
set. Since enable/disable properties share the same enum value, they can
be trivially checked via the newly added bitmap. Having both enable and
disable for the same config makes no sense at all, so produce an error
in this case.

For interactions between properties, doing them outside the loop makes
more sense as it can be evaluated once. In case there are some edge
cases that would be broken by producing an error, only warn for now.

Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
---
ngl, this is a bit of a lazy approach, and I just didn't bother checking
the more complex things that a simple bitmap scheme could not address.
Chief among the more complexes thing that I didn't do anything about is
that properties that I don't feel make sense when something is disabled
don't complain when they are present but the disabled property also is.
That's because enable and disable are the same bit in the bitmap, so
more info than just the bitmap would be required to perform that check.

Part of me says that that's okay and should be handled by dt tools, since
it's mostly harmless and that denying things that are effectively
undefined behaviour (well really, they're driver/implementation defined
behaviour) is what's actually valuable here.

I dunno about the tests being done as a sum, but I dunno if there's a
neater way to do that test without creating bitmaps to and with the
created one.
---
 drivers/pinctrl/pinconf-generic.c | 41 ++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 366775841c63..d182ec84e2df 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -222,7 +222,10 @@ static int parse_dt_cfg(struct device_node *np,
 			unsigned int count, unsigned long *cfg,
 			unsigned int *ncfg)
 {
-	int i;
+	unsigned long *properties;
+	int i, test;
+
+	properties = bitmap_zalloc(count, GFP_KERNEL);
 
 	for (i = 0; i < count; i++) {
 		u32 val;
@@ -251,11 +254,45 @@ static int parse_dt_cfg(struct device_node *np,
 		if (ret)
 			val = par->default_value;
 
+		/* if param is greater than count, these are custom properties */
+		if (par->param <= count) {
+			ret = test_and_set_bit(par->param, properties);
+			if (ret) {
+				pr_err("%s: conflicting setting detected for %s\n",
+				       np->name, par->property);
+				bitmap_free(properties);
+				return -EINVAL;
+			}
+		}
+
 		pr_debug("found %s with value %u\n", par->property, val);
 		cfg[*ncfg] = pinconf_to_config_packed(par->param, val);
 		(*ncfg)++;
 	}
 
+	if (test_bit(PIN_CONFIG_DRIVE_STRENGTH, properties) &&
+			test_bit(PIN_CONFIG_DRIVE_STRENGTH_UA, properties))
+		pr_err("%s: cannot have multiple drive strength properties\n",
+		       np->name);
+
+	test = test_bit(PIN_CONFIG_BIAS_BUS_HOLD, properties) +
+		test_bit(PIN_CONFIG_BIAS_DISABLE, properties) +
+		test_bit(PIN_CONFIG_BIAS_HIGH_IMPEDANCE, properties) +
+		test_bit(PIN_CONFIG_BIAS_PULL_UP, properties) +
+		test_bit(PIN_CONFIG_BIAS_PULL_PIN_DEFAULT, properties) +
+		test_bit(PIN_CONFIG_BIAS_PULL_DOWN, properties);
+	if (test > 1)
+		pr_err("%s: cannot have multiple bias configurations\n",
+		       np->name);
+
+	test = test_bit(PIN_CONFIG_DRIVE_OPEN_DRAIN, properties) +
+		test_bit(PIN_CONFIG_DRIVE_OPEN_SOURCE, properties) +
+		test_bit(PIN_CONFIG_DRIVE_PUSH_PULL, properties);
+	if (test > 1)
+		pr_err("%s: cannot have multiple drive configurations\n",
+		       np->name);
+
+	bitmap_free(properties);
 	return 0;
 }
 
@@ -352,6 +389,7 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
 	ret = parse_dt_cfg(np, dt_params, ARRAY_SIZE(dt_params), cfg, &ncfg);
 	if (ret)
 		return ret;
+
 	if (pctldev && pctldev->desc->num_custom_params &&
 		pctldev->desc->custom_params) {
 		ret = parse_dt_cfg(np, pctldev->desc->custom_params,
@@ -360,6 +398,7 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
 			return ret;
 	}
 
+
 	/* no configs found at all */
 	if (ncfg == 0) {
 		*configs = NULL;
-- 
2.51.0