Re: [PATCH v3 4/5] security: configure pinctrl based on policy name

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 3/18/26 10:22, Fabian Pflug wrote:
> When using security policies to disable console input on the default
> console, it might be more advantagous to also disable the RX pin hard
> in pinctrl, so that if there is a software error with the security
> policy implementation input does not reach to system and cannot be
> exploited.
> 
> An example devicetree could look like this:
> / {
> 	chosen {
> 		stdout-path = &uart3;
> 	};
> };
> 
> &uart3 {
> 	pinctrl-names = "default", "barebox,policy-devel";
> 	pinctrl-0 = <&pinctrl_uart3_tx_only>;
> 	pinctrl-1 = <&pinctrl_uart3_interactive>;
> 	status = "okay";
> };
> 
> &iomuxc {
> 	pinctrl_uart3_interactive: uart3ingrp {
> 		fsl,pins = <MX8MP_IOMUXC_SD1_DATA6__UART3_DCE_TX	0x140>,
> 			   <MX8MP_IOMUXC_SD1_DATA7__UART3_DCE_RX	0x140>;
> 	};
> 
> 	pinctrl_uart3_tx_only: uart3txgrp {
> 		fsl,pins = <MX8MP_IOMUXC_SD1_DATA6__UART3_DCE_TX	0x140>,
> 			   <MX8MP_IOMUXC_SD1_DATA7__GPIO2_IO09		0x140>;
> 	};
> };
> 
> This would apply the devel pinmux on selecting the devel config and the
> default on every other configuration.

The barebox,policy- pattern should be documented in Documentation/devicetree/bindings/

> -	pinctrl_select_state_default(dev);
> +
> +	if (IS_ENABLED(CONFIG_SECURITY_POLICY_PINCTRL)) {
> +		char *policy_pinctrl;
> +
> +		policy_pinctrl = basprintf("barebox,policy-%s", active_policy->name);
> +		if (IS_ERR(pinctrl_get_select(dev, policy_pinctrl)))
> +			pinctrl_select_state_default(dev);
> +		free(policy_pinctrl);
> +	} else
> +		pinctrl_select_state_default(dev);

I think this logic should go somewhere so that pinctrl_select_state_default
can make use of it as there are drivers that call
pinctrl_select_state_default() tha would be unaffected.

Also kernel coding stye is to use braces in else if the if clause
has them.

>  	of_clk_set_defaults(dev->of_node, false);
>  
>  	list_add(&dev->active, &active_device_list);
> diff --git a/security/Kconfig.policy b/security/Kconfig.policy
> index 9ea52e91da..8ddb67ac2d 100644
> --- a/security/Kconfig.policy
> +++ b/security/Kconfig.policy
> @@ -68,6 +68,14 @@ config SECURITY_POLICY_DEFAULT_PERMISSIVE
>  	  A security policy should always be selected, either early on by
>  	  board code or via CONFIG_SECURITY_POLICY_INIT.
>  
> +config SECURITY_POLICY_PINCTRL
> +	bool "Update pinctrl based on policy-name"
> +	help
> +	  Changing the security policy, will look for a pinctrl with the name
> +	  barebox,policy-<policyname>. If there is one, it will change the
> +	  pinctrl for this. This could be used to disable the RX (and TX)
> +	  Pin in lockdown mode for the console or disable the usage of SPI.

"for example".

> +
>  config SECURITY_POLICY_PATH
>  	string
>  	depends on SECURITY_POLICY
> diff --git a/security/policy.c b/security/policy.c
> index e2d1b10a78..4d51af63e7 100644
> --- a/security/policy.c
> +++ b/security/policy.c
> @@ -7,6 +7,7 @@
>  #include <linux/bitmap.h>
>  #include <param.h>
>  #include <device.h>
> +#include <pinctrl.h>
>  #include <stdio.h>
>  
>  #include <security/policy.h>
> @@ -90,12 +91,23 @@ bool is_allowed(const struct security_policy *policy, unsigned option)
>  int security_policy_activate(const struct security_policy *policy)
>  {
>  	const struct security_policy *old_policy = active_policy;
> +	struct device *dev;
> +	char *policy_pinctrl;
>  
>  	if (policy == old_policy)
>  		return 0;
>  
>  	active_policy = policy;
>  
> +	if (IS_ENABLED(CONFIG_SECURITY_POLICY_PINCTRL)) {
> +		policy_pinctrl = basprintf("barebox,policy-%s", active_policy->name);
> +		list_for_each_entry(dev, &active_device_list, active) {
> +			if (IS_ERR(pinctrl_get_select(dev, policy_pinctrl)))
> +				pinctrl_select_state_default(dev);
> +		}
> +		free(policy_pinctrl);
> +	}

This breaks HS200 on i.MX. Reverting the default state on error is not
a safe fallback.

I am thinking maybe, we should change the binding a bit and make it:

barebox,policy-lockdown-default for example and then a lookup
for "default" also does a lookup for "barebox,policy-${policy}-default".

That way, we will have something generically usable.

In any case, it's not acceptable to change pinctrl settings of devices
that did not have security policy specific pinctrl groups.

Thanks,
Ahmad

> +
>  	for (int i = 0; i < SCONFIG_NUM; i++) {
>  		if (__is_allowed(policy, i) == __is_allowed(old_policy, i))
>  			continue;
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |