* [PATCH v3 1/5] of: add of_property_write_string_array()
2026-03-18 9:21 [PATCH v3 0/5] Add helper for security policies Fabian Pflug
@ 2026-03-18 9:21 ` Fabian Pflug
2026-03-18 9:22 ` [PATCH v3 2/5] common: bootm: add policy to commandline Fabian Pflug
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Fabian Pflug @ 2026-03-18 9:21 UTC (permalink / raw)
To: BAREBOX, Sascha Hauer; +Cc: Fabian Pflug
Helper function to write an array of string into a property.
Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
drivers/of/base.c | 43 +++++++++++++++++++++++++++++++++++++++++++
include/of.h | 3 +++
test/self/of_manipulation.c | 14 +++++++++++++-
test/self/of_manipulation.dts | 5 +++++
4 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 460b5e2f4b..376a2ac6a8 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1570,6 +1570,49 @@ int of_property_write_u64_array(struct device_node *np,
return 0;
}
+/**
+ * of_property_write_string_array - Write strings to a property. If
+ * the property does not exist, it will be created and appended to the given
+ * device node.
+ *
+ * @np: device node to which the property value is to be written.
+ * @propname: name of the property to be written.
+ * @values: pointer to array elements to write.
+ * @sz: number of array elements to write.
+ *
+ * Search for a property in a device node and write a string to
+ * it. If the property does not exist, it will be created and appended to
+ * the device node. Returns 0 on success, -ENOMEM if the property or array
+ * of elements cannot be created, -EINVAL if no strings specified.
+ */
+int of_property_write_string_array(struct device_node *np,
+ const char *propname, const char **values,
+ size_t sz)
+{
+ char *buf = NULL, *next;
+ size_t len = 0;
+ int ret = 0, i;
+
+ for (i = 0; i < sz; i++)
+ len += strlen(values[i]) + 1;
+
+ if (!len)
+ return -EINVAL;
+
+ buf = malloc(len);
+ if (!buf)
+ return -ENOMEM;
+
+ next = buf;
+
+ for (i = 0; i < sz; i++)
+ next = stpcpy(next, values[i]) + 1;
+
+ ret = of_set_property(np, propname, buf, len, 1);
+ free(buf);
+ return ret;
+}
+
/**
* of_property_write_strings - Write strings to a property. If
* the property does not exist, it will be created and appended to the given
diff --git a/include/of.h b/include/of.h
index ba8d1e358d..34439ee763 100644
--- a/include/of.h
+++ b/include/of.h
@@ -310,6 +310,9 @@ extern int of_property_write_string(struct device_node *np, const char *propname
const char *value);
extern int of_property_write_strings(struct device_node *np, const char *propname,
...) __attribute__((__sentinel__));
+int of_property_write_string_array(struct device_node *np,
+ const char *propname, const char **values,
+ size_t sz);
int of_property_sprintf(struct device_node *np, const char *propname, const char *fmt, ...)
__printf(3, 4);
diff --git a/test/self/of_manipulation.c b/test/self/of_manipulation.c
index 8d645b1137..faf948a17b 100644
--- a/test/self/of_manipulation.c
+++ b/test/self/of_manipulation.c
@@ -71,13 +71,19 @@ static void test_of_basics(struct device_node *root)
static void test_of_property_strings(struct device_node *root)
{
- struct device_node *np1, *np2, *np3, *np4;
+ struct device_node *np1, *np2, *np3, *np4, *np5;
char properties[] = "ayy\0bee\0sea";
+ static const char * const prop_array[] = {
+ "ayy",
+ "bee\0\0\0",
+ "sea\0",
+ };
np1 = of_new_node(root, "np1");
np2 = of_new_node(root, "np2");
np3 = of_new_node(root, "np3");
np4 = of_new_node(root, "np4");
+ np5 = of_new_node(root, "np5");
of_property_sprintf(np1, "property-single", "%c%c%c", 'a', 'y', 'y');
@@ -108,6 +114,12 @@ static void test_of_property_strings(struct device_node *root)
of_prepend_property(np4, "property-multi", "ayy", 4);
assert_equal(np3, np4);
+
+ of_property_write_string_array(np5, "property-single", prop_array, 1);
+
+ of_property_write_string_array(np5, "property-multi", prop_array, 3);
+
+ assert_equal(np4, np5);
}
static void __init test_of_manipulation(void)
diff --git a/test/self/of_manipulation.dts b/test/self/of_manipulation.dts
index 2cc6773fa9..7a1fb1217d 100644
--- a/test/self/of_manipulation.dts
+++ b/test/self/of_manipulation.dts
@@ -34,4 +34,9 @@ np4 {
property-single = "ayy";
property-multi = "ayy", "bee", "sea";
};
+
+ np5 {
+ property-single = "ayy";
+ property-multi = "ayy", "bee", "sea";
+ };
};
--
2.47.3
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v3 2/5] common: bootm: add policy to commandline
2026-03-18 9:21 [PATCH v3 0/5] Add helper for security policies Fabian Pflug
2026-03-18 9:21 ` [PATCH v3 1/5] of: add of_property_write_string_array() Fabian Pflug
@ 2026-03-18 9:22 ` Fabian Pflug
2026-03-18 10:23 ` Sascha Hauer
2026-03-18 9:22 ` [PATCH v3 3/5] security: policy: set active policy on boot Fabian Pflug
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Fabian Pflug @ 2026-03-18 9:22 UTC (permalink / raw)
To: BAREBOX, Sascha Hauer; +Cc: Fabian Pflug
If security policies are used, then the variable bootm.provide_policy
can be set to automatically append the currently selected security
policy to the kernel commandline with the prefix
barebox.security.policy=
This allows the the system to behave different based on the selected
security policy.
Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
common/bootm.c | 23 +++++++++++++++++++++++
include/bootm.h | 5 +++++
2 files changed, 28 insertions(+)
diff --git a/common/bootm.c b/common/bootm.c
index d43079bb81..9484539bc3 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -22,6 +22,7 @@
#include <uncompress.h>
#include <zero_page.h>
#include <security/config.h>
+#include <security/policy.h>
static LIST_HEAD(handler_list);
static struct sconfig_notifier_block sconfig_notifier;
@@ -75,6 +76,7 @@ static int bootm_dryrun;
static int bootm_earlycon;
static int bootm_provide_machine_id;
static int bootm_provide_hostname;
+static int bootm_provide_policy;
static int bootm_verbosity;
static int bootm_efi_mode = BOOTM_EFI_AVAILABLE;
@@ -97,6 +99,7 @@ void bootm_data_init_defaults(struct bootm_data *data)
data->appendroot = bootm_appendroot;
data->provide_machine_id = bootm_provide_machine_id;
data->provide_hostname = bootm_provide_hostname;
+ data->provide_policy = bootm_provide_policy;
data->verbose = bootm_verbosity;
data->dryrun = bootm_dryrun;
data->efi_boot = bootm_efi_mode;
@@ -118,6 +121,7 @@ void bootm_data_restore_defaults(const struct bootm_data *data)
bootm_appendroot = data->appendroot;
bootm_provide_machine_id = data->provide_machine_id;
bootm_provide_hostname = data->provide_hostname;
+ bootm_provide_policy = data->provide_policy;
bootm_verbosity = data->verbose;
bootm_dryrun = data->dryrun;
bootm_efi_mode = data->efi_boot;
@@ -759,6 +763,20 @@ int bootm_boot(struct bootm_data *bootm_data)
free(hostname_bootarg);
}
+ if (IS_ENABLED(CONFIG_SECURITY_POLICY) && bootm_data->provide_policy) {
+ char *policy_bootargs;
+
+ if (active_policy && !active_policy->name) {
+ pr_err("Providing policy is enabled but policy has no name\n");
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ policy_bootargs = basprintf("barebox.security.policy=%s", active_policy->name);
+ globalvar_add_simple("linux.bootargs.dyn.policy", policy_bootargs);
+ free(policy_bootargs);
+ }
+
pr_info("\nLoading %s '%s'", file_type_to_string(data->kernel_type),
data->os_file);
if (data->kernel_type == filetype_uimage &&
@@ -967,6 +985,8 @@ static int bootm_init(void)
globalvar_add_simple_bool("bootm.earlycon", &bootm_earlycon);
globalvar_add_simple_bool("bootm.provide_machine_id", &bootm_provide_machine_id);
globalvar_add_simple_bool("bootm.provide_hostname", &bootm_provide_hostname);
+ if (IS_ENABLED(CONFIG_SECURITY_POLICY))
+ globalvar_add_simple_bool("bootm.provide_policy", &bootm_provide_policy);
if (IS_ENABLED(CONFIG_BOOTM_INITRD)) {
globalvar_add_simple("bootm.initrd", NULL);
globalvar_add_simple("bootm.initrd.loadaddr", NULL);
@@ -1030,3 +1050,6 @@ BAREBOX_MAGICVAR(global.bootm.root_dev, "bootm default root device (overrides de
BAREBOX_MAGICVAR(global.bootm.root_param, "bootm root parameter name (normally 'root' for root=/dev/...)");
BAREBOX_MAGICVAR(global.bootm.provide_machine_id, "If true, append systemd.machine_id=$global.machine_id to Kernel command line");
BAREBOX_MAGICVAR(global.bootm.provide_hostname, "If true, append systemd.hostname=$global.hostname to Kernel command line");
+#ifdef CONFIG_SECURITY_POLICY
+BAREBOX_MAGICVAR(global.bootm.provide_policy, "Add barebox.security.policy= option to Kernel");
+#endif
diff --git a/include/bootm.h b/include/bootm.h
index 21feb1ca98..a712010b2b 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -46,6 +46,11 @@ struct bootm_data {
* of global.hostname to Kernel.
*/
bool provide_hostname;
+ /*
+ * provide_policy - if true, try to add barebox.security.policy= with
+ * with value of currently selected policy
+ */
+ bool provide_policy;
enum bootm_efi_mode efi_boot;
unsigned long initrd_address;
unsigned long os_address;
--
2.47.3
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 2/5] common: bootm: add policy to commandline
2026-03-18 9:22 ` [PATCH v3 2/5] common: bootm: add policy to commandline Fabian Pflug
@ 2026-03-18 10:23 ` Sascha Hauer
0 siblings, 0 replies; 16+ messages in thread
From: Sascha Hauer @ 2026-03-18 10:23 UTC (permalink / raw)
To: Fabian Pflug; +Cc: BAREBOX, Ahmad Fatoum
On Wed, Mar 18, 2026 at 10:22:00AM +0100, Fabian Pflug wrote:
> If security policies are used, then the variable bootm.provide_policy
> can be set to automatically append the currently selected security
> policy to the kernel commandline with the prefix
> barebox.security.policy=
> This allows the the system to behave different based on the selected
> security policy.
>
> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
> ---
> common/bootm.c | 23 +++++++++++++++++++++++
> include/bootm.h | 5 +++++
> 2 files changed, 28 insertions(+)
>
> diff --git a/common/bootm.c b/common/bootm.c
> index d43079bb81..9484539bc3 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -22,6 +22,7 @@
> #include <uncompress.h>
> #include <zero_page.h>
> #include <security/config.h>
> +#include <security/policy.h>
>
> static LIST_HEAD(handler_list);
> static struct sconfig_notifier_block sconfig_notifier;
> @@ -75,6 +76,7 @@ static int bootm_dryrun;
> static int bootm_earlycon;
> static int bootm_provide_machine_id;
> static int bootm_provide_hostname;
> +static int bootm_provide_policy;
> static int bootm_verbosity;
> static int bootm_efi_mode = BOOTM_EFI_AVAILABLE;
>
> @@ -97,6 +99,7 @@ void bootm_data_init_defaults(struct bootm_data *data)
> data->appendroot = bootm_appendroot;
> data->provide_machine_id = bootm_provide_machine_id;
> data->provide_hostname = bootm_provide_hostname;
> + data->provide_policy = bootm_provide_policy;
> data->verbose = bootm_verbosity;
> data->dryrun = bootm_dryrun;
> data->efi_boot = bootm_efi_mode;
> @@ -118,6 +121,7 @@ void bootm_data_restore_defaults(const struct bootm_data *data)
> bootm_appendroot = data->appendroot;
> bootm_provide_machine_id = data->provide_machine_id;
> bootm_provide_hostname = data->provide_hostname;
> + bootm_provide_policy = data->provide_policy;
> bootm_verbosity = data->verbose;
> bootm_dryrun = data->dryrun;
> bootm_efi_mode = data->efi_boot;
> @@ -759,6 +763,20 @@ int bootm_boot(struct bootm_data *bootm_data)
> free(hostname_bootarg);
> }
>
> + if (IS_ENABLED(CONFIG_SECURITY_POLICY) && bootm_data->provide_policy) {
This change conflicts with Ahmads bootm override series. In that series
int bootm_boot(struct bootm_data *bootm_data)
is replaced with
int bootm_boot_handler(struct image_data *data)
So bootm_data is no longer available here. I replaced bootm_data->provide_policy
with 0 for now, but this needs a proper fix. Ahmad, Fabian, Could you look into it?
Sascha
--
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 |
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 3/5] security: policy: set active policy on boot
2026-03-18 9:21 [PATCH v3 0/5] Add helper for security policies Fabian Pflug
2026-03-18 9:21 ` [PATCH v3 1/5] of: add of_property_write_string_array() Fabian Pflug
2026-03-18 9:22 ` [PATCH v3 2/5] common: bootm: add policy to commandline Fabian Pflug
@ 2026-03-18 9:22 ` Fabian Pflug
2026-03-18 11:28 ` Ahmad Fatoum
2026-03-18 9:22 ` [PATCH v3 4/5] security: configure pinctrl based on policy name Fabian Pflug
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Fabian Pflug @ 2026-03-18 9:22 UTC (permalink / raw)
To: BAREBOX, Sascha Hauer; +Cc: Fabian Pflug
If init name has been set at compiletime and the policy is available,
because it is part of the path, then set the active policy to the policy
selected by compiletime.
Since this is so early in the bootchain, there is no need to call
security_policy_activate, because there should not be any registered
callbacks at this moment in time.
If no policy could be found, then it will be filled as before by the
first call to is_allowed.
Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
security/policy.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/security/policy.c b/security/policy.c
index 85333d9e6f..e2d1b10a78 100644
--- a/security/policy.c
+++ b/security/policy.c
@@ -235,6 +235,9 @@ static int security_init(void)
if (*CONFIG_SECURITY_POLICY_PATH)
security_policy_add(default);
+ if (*CONFIG_SECURITY_POLICY_INIT)
+ active_policy = security_policy_get(CONFIG_SECURITY_POLICY_INIT);
+
return 0;
}
pure_initcall(security_init);
--
2.47.3
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/5] security: policy: set active policy on boot
2026-03-18 9:22 ` [PATCH v3 3/5] security: policy: set active policy on boot Fabian Pflug
@ 2026-03-18 11:28 ` Ahmad Fatoum
2026-03-18 11:38 ` Fabian Pflug
0 siblings, 1 reply; 16+ messages in thread
From: Ahmad Fatoum @ 2026-03-18 11:28 UTC (permalink / raw)
To: Fabian Pflug, BAREBOX, Sascha Hauer
On 3/18/26 10:22, Fabian Pflug wrote:
> If init name has been set at compiletime and the policy is available,
> because it is part of the path, then set the active policy to the policy
> selected by compiletime.
> Since this is so early in the bootchain, there is no need to call
> security_policy_activate, because there should not be any registered
> callbacks at this moment in time.
> If no policy could be found, then it will be filled as before by the
> first call to is_allowed.
The code in is_allowed is:
if (!policy && *CONFIG_SECURITY_POLICY_INIT) {
security_policy_select(CONFIG_SECURITY_POLICY_INIT);
policy = active_policy;
}
It becomes dead code with your change here as CONFIG_SECURITY_POLICY_INIT
is a compile-time constant, there is no filling on the first call anymore.
>
> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
> ---
> security/policy.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/security/policy.c b/security/policy.c
> index 85333d9e6f..e2d1b10a78 100644
> --- a/security/policy.c
> +++ b/security/policy.c
> @@ -235,6 +235,9 @@ static int security_init(void)
> if (*CONFIG_SECURITY_POLICY_PATH)
> security_policy_add(default);
>
> + if (*CONFIG_SECURITY_POLICY_INIT)
> + active_policy = security_policy_get(CONFIG_SECURITY_POLICY_INIT);
> +
I think I decided initially against this, because there was initially
a Sconfig option against changing the active security policy.
I believe now a single option is too limiting, it should instead be
a directed graph that explains which policies are reachable from a given
policy.
Anyways, the change here invalidates the Kconfig help text for
SECURITY_POLICY_INIT.
I am not fully sure if this change is a good idea, but it needs to
be fixed to be considered. I assume you do this, because checking
the name of the policy doesn't trigger a selection like IS_ALLOWED does?
Thanks,
Ahmad
> return 0;
> }
> pure_initcall(security_init);
>
--
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 |
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/5] security: policy: set active policy on boot
2026-03-18 11:28 ` Ahmad Fatoum
@ 2026-03-18 11:38 ` Fabian Pflug
2026-03-18 11:54 ` Ahmad Fatoum
0 siblings, 1 reply; 16+ messages in thread
From: Fabian Pflug @ 2026-03-18 11:38 UTC (permalink / raw)
To: Ahmad Fatoum, BAREBOX, Sascha Hauer
On Wed, 2026-03-18 at 12:28 +0100, Ahmad Fatoum wrote:
> On 3/18/26 10:22, Fabian Pflug wrote:
> > If init name has been set at compiletime and the policy is available,
> > because it is part of the path, then set the active policy to the policy
> > selected by compiletime.
> > Since this is so early in the bootchain, there is no need to call
> > security_policy_activate, because there should not be any registered
> > callbacks at this moment in time.
> > If no policy could be found, then it will be filled as before by the
> > first call to is_allowed.
>
> The code in is_allowed is:
>
> if (!policy && *CONFIG_SECURITY_POLICY_INIT) {
> security_policy_select(CONFIG_SECURITY_POLICY_INIT);
> policy = active_policy;
> }
>
> It becomes dead code with your change here as CONFIG_SECURITY_POLICY_INIT
> is a compile-time constant, there is no filling on the first call anymore.
I also thought about it, but if the initial policy is not part of the compiletime policies, but instead gets added
during board setup code, then the change in init will not find the specified policy, resulting in policy being NULL and
this code still working.
>
> >
> > Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
> > ---
> > security/policy.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/security/policy.c b/security/policy.c
> > index 85333d9e6f..e2d1b10a78 100644
> > --- a/security/policy.c
> > +++ b/security/policy.c
> > @@ -235,6 +235,9 @@ static int security_init(void)
> > if (*CONFIG_SECURITY_POLICY_PATH)
> > security_policy_add(default);
> >
> > + if (*CONFIG_SECURITY_POLICY_INIT)
> > + active_policy = security_policy_get(CONFIG_SECURITY_POLICY_INIT);
> > +
>
> I think I decided initially against this, because there was initially
> a Sconfig option against changing the active security policy.
>
> I believe now a single option is too limiting, it should instead be
> a directed graph that explains which policies are reachable from a given
> policy.
>
> Anyways, the change here invalidates the Kconfig help text for
> SECURITY_POLICY_INIT.
>
> I am not fully sure if this change is a good idea, but it needs to
> be fixed to be considered. I assume you do this, because checking
> the name of the policy doesn't trigger a selection like IS_ALLOWED does?
exactly.
during device_probe, there is a need to know the current policy name, if there is a policy active.
I will have a look into it.
Fabian
>
> Thanks,
> Ahmad
>
>
> > return 0;
> > }
> > pure_initcall(security_init);
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/5] security: policy: set active policy on boot
2026-03-18 11:38 ` Fabian Pflug
@ 2026-03-18 11:54 ` Ahmad Fatoum
2026-03-18 12:47 ` Fabian Pflug
0 siblings, 1 reply; 16+ messages in thread
From: Ahmad Fatoum @ 2026-03-18 11:54 UTC (permalink / raw)
To: Fabian Pflug, BAREBOX, Sascha Hauer
On 3/18/26 12:38, Fabian Pflug wrote:
> On Wed, 2026-03-18 at 12:28 +0100, Ahmad Fatoum wrote:
>> On 3/18/26 10:22, Fabian Pflug wrote:
>>> If init name has been set at compiletime and the policy is available,
>>> because it is part of the path, then set the active policy to the policy
>>> selected by compiletime.
>>> Since this is so early in the bootchain, there is no need to call
>>> security_policy_activate, because there should not be any registered
>>> callbacks at this moment in time.
>>> If no policy could be found, then it will be filled as before by the
>>> first call to is_allowed.
>>
>> The code in is_allowed is:
>>
>> if (!policy && *CONFIG_SECURITY_POLICY_INIT) {
>> security_policy_select(CONFIG_SECURITY_POLICY_INIT);
>> policy = active_policy;
>> }
>>
>> It becomes dead code with your change here as CONFIG_SECURITY_POLICY_INIT
>> is a compile-time constant, there is no filling on the first call anymore.
>
> I also thought about it, but if the initial policy is not part of the compiletime policies, but instead gets added
> during board setup code, then the change in init will not find the specified policy, resulting in policy being NULL and
> this code still working.
I can't follow. policy is an argument and CONFIG_SECURITY_POLICY_INIT
is not settable from any board, so that's dead code now AFAICS.
Cheers,
Ahmad
>
>>
>>>
>>> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
>>> ---
>>> security/policy.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/security/policy.c b/security/policy.c
>>> index 85333d9e6f..e2d1b10a78 100644
>>> --- a/security/policy.c
>>> +++ b/security/policy.c
>>> @@ -235,6 +235,9 @@ static int security_init(void)
>>> if (*CONFIG_SECURITY_POLICY_PATH)
>>> security_policy_add(default);
>>>
>>> + if (*CONFIG_SECURITY_POLICY_INIT)
>>> + active_policy = security_policy_get(CONFIG_SECURITY_POLICY_INIT);
>>> +
>>
>> I think I decided initially against this, because there was initially
>> a Sconfig option against changing the active security policy.
>>
>> I believe now a single option is too limiting, it should instead be
>> a directed graph that explains which policies are reachable from a given
>> policy.
>>
>> Anyways, the change here invalidates the Kconfig help text for
>> SECURITY_POLICY_INIT.
>>
>> I am not fully sure if this change is a good idea, but it needs to
>> be fixed to be considered. I assume you do this, because checking
>> the name of the policy doesn't trigger a selection like IS_ALLOWED does?
>
> exactly.
> during device_probe, there is a need to know the current policy name, if there is a policy active.
>
> I will have a look into it.
>
> Fabian
>
>>
>> Thanks,
>> Ahmad
>>
>>
>>> return 0;
>>> }
>>> pure_initcall(security_init);
>>>
>>
>
--
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 |
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/5] security: policy: set active policy on boot
2026-03-18 11:54 ` Ahmad Fatoum
@ 2026-03-18 12:47 ` Fabian Pflug
2026-03-19 14:58 ` Ahmad Fatoum
0 siblings, 1 reply; 16+ messages in thread
From: Fabian Pflug @ 2026-03-18 12:47 UTC (permalink / raw)
To: Ahmad Fatoum, BAREBOX, Sascha Hauer
On Wed, 2026-03-18 at 12:54 +0100, Ahmad Fatoum wrote:
> On 3/18/26 12:38, Fabian Pflug wrote:
> > On Wed, 2026-03-18 at 12:28 +0100, Ahmad Fatoum wrote:
> > > On 3/18/26 10:22, Fabian Pflug wrote:
> > > > If init name has been set at compiletime and the policy is available,
> > > > because it is part of the path, then set the active policy to the policy
> > > > selected by compiletime.
> > > > Since this is so early in the bootchain, there is no need to call
> > > > security_policy_activate, because there should not be any registered
> > > > callbacks at this moment in time.
> > > > If no policy could be found, then it will be filled as before by the
> > > > first call to is_allowed.
> > >
> > > The code in is_allowed is:
> > >
> > > if (!policy && *CONFIG_SECURITY_POLICY_INIT) {
> > > security_policy_select(CONFIG_SECURITY_POLICY_INIT);
> > > policy = active_policy;
> > > }
> > >
> > > It becomes dead code with your change here as CONFIG_SECURITY_POLICY_INIT
> > > is a compile-time constant, there is no filling on the first call anymore.
> >
> > I also thought about it, but if the initial policy is not part of the compiletime policies, but instead gets added
> > during board setup code, then the change in init will not find the specified policy, resulting in policy being NULL
> > and
> > this code still working.
>
> I can't follow. policy is an argument and CONFIG_SECURITY_POLICY_INIT
> is not settable from any board, so that's dead code now AFAICS.
policy is the argument, but the argument could be NULL, for example if `IS_ALLOWED` is used in the code.
Then policy is replaced by active_policy, which could also be NULL, if `security_policy_get(CONFIG_SECURITY_POLICY_INIT)
` during init returns NULL, which is the case, if the policy is not registered at the time of call.
During security_init only CONFIG_SECURITY_POLICY_PATH are registered. So for example, you could add multiple policys
with `security_policy_add` inside your boardcode and have one of them declared as init policy with
CONFIG_SECURITY_POLICY_INIT. Then this path is taken during the first call to `IS_ALLOWED` (after board init)
Kind regards
Fabian
>
> Cheers,
> Ahmad
>
> >
> > >
> > > >
> > > > Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
> > > > ---
> > > > security/policy.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/security/policy.c b/security/policy.c
> > > > index 85333d9e6f..e2d1b10a78 100644
> > > > --- a/security/policy.c
> > > > +++ b/security/policy.c
> > > > @@ -235,6 +235,9 @@ static int security_init(void)
> > > > if (*CONFIG_SECURITY_POLICY_PATH)
> > > > security_policy_add(default);
> > > >
> > > > + if (*CONFIG_SECURITY_POLICY_INIT)
> > > > + active_policy = security_policy_get(CONFIG_SECURITY_POLICY_INIT);
> > > > +
> > >
> > > I think I decided initially against this, because there was initially
> > > a Sconfig option against changing the active security policy.
> > >
> > > I believe now a single option is too limiting, it should instead be
> > > a directed graph that explains which policies are reachable from a given
> > > policy.
> > >
> > > Anyways, the change here invalidates the Kconfig help text for
> > > SECURITY_POLICY_INIT.
> > >
> > > I am not fully sure if this change is a good idea, but it needs to
> > > be fixed to be considered. I assume you do this, because checking
> > > the name of the policy doesn't trigger a selection like IS_ALLOWED does?
> >
> > exactly.
> > during device_probe, there is a need to know the current policy name, if there is a policy active.
> >
> > I will have a look into it.
> >
> > Fabian
> >
> > >
> > > Thanks,
> > > Ahmad
> > >
> > >
> > > > return 0;
> > > > }
> > > > pure_initcall(security_init);
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 3/5] security: policy: set active policy on boot
2026-03-18 12:47 ` Fabian Pflug
@ 2026-03-19 14:58 ` Ahmad Fatoum
0 siblings, 0 replies; 16+ messages in thread
From: Ahmad Fatoum @ 2026-03-19 14:58 UTC (permalink / raw)
To: Fabian Pflug, BAREBOX, Sascha Hauer
Hi,
On 3/18/26 1:47 PM, Fabian Pflug wrote:
> On Wed, 2026-03-18 at 12:54 +0100, Ahmad Fatoum wrote:
>> On 3/18/26 12:38, Fabian Pflug wrote:
>>> On Wed, 2026-03-18 at 12:28 +0100, Ahmad Fatoum wrote:
>>>> On 3/18/26 10:22, Fabian Pflug wrote:
>>>>> If init name has been set at compiletime and the policy is available,
>>>>> because it is part of the path, then set the active policy to the policy
>>>>> selected by compiletime.
>>>>> Since this is so early in the bootchain, there is no need to call
>>>>> security_policy_activate, because there should not be any registered
>>>>> callbacks at this moment in time.
>>>>> If no policy could be found, then it will be filled as before by the
>>>>> first call to is_allowed.
>>>>
>>>> The code in is_allowed is:
>>>>
>>>> if (!policy && *CONFIG_SECURITY_POLICY_INIT) {
>>>> security_policy_select(CONFIG_SECURITY_POLICY_INIT);
>>>> policy = active_policy;
>>>> }
>>>>
>>>> It becomes dead code with your change here as CONFIG_SECURITY_POLICY_INIT
>>>> is a compile-time constant, there is no filling on the first call anymore.
>>>
>>> I also thought about it, but if the initial policy is not part of the compiletime policies, but instead gets added
>>> during board setup code, then the change in init will not find the specified policy, resulting in policy being NULL
>>> and
>>> this code still working.
>>
>> I can't follow. policy is an argument and CONFIG_SECURITY_POLICY_INIT
>> is not settable from any board, so that's dead code now AFAICS.
>
> policy is the argument, but the argument could be NULL, for example if `IS_ALLOWED` is used in the code.
> Then policy is replaced by active_policy, which could also be NULL, if `security_policy_get(CONFIG_SECURITY_POLICY_INIT)
> ` during init returns NULL, which is the case, if the policy is not registered at the time of call.
> During security_init only CONFIG_SECURITY_POLICY_PATH are registered. So for example, you could add multiple policys
> with `security_policy_add` inside your boardcode and have one of them declared as init policy with
> CONFIG_SECURITY_POLICY_INIT. Then this path is taken during the first call to `IS_ALLOWED` (after board init)
I talked it over with Fabian. I see now what I misunderstood: The policy
is selected only if it exists. If it's not registered yet, we still need
the later policy select.
Still, I feel this complicates the logic more than I'd like.
Instead we can factor out the CONFIG_SECURITY_POLICY_INIT logic in
is_allowed into security_policy_ensure (or some better name) and then
call that explicitly on entry to functions that expect the policy
selection to be settled.
Cheers,
Ahmad
>
> Kind regards
> Fabian
>>
>> Cheers,
>> Ahmad
>>
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
>>>>> ---
>>>>> security/policy.c | 3 +++
>>>>> 1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/security/policy.c b/security/policy.c
>>>>> index 85333d9e6f..e2d1b10a78 100644
>>>>> --- a/security/policy.c
>>>>> +++ b/security/policy.c
>>>>> @@ -235,6 +235,9 @@ static int security_init(void)
>>>>> if (*CONFIG_SECURITY_POLICY_PATH)
>>>>> security_policy_add(default);
>>>>>
>>>>> + if (*CONFIG_SECURITY_POLICY_INIT)
>>>>> + active_policy = security_policy_get(CONFIG_SECURITY_POLICY_INIT);
>>>>> +
>>>>
>>>> I think I decided initially against this, because there was initially
>>>> a Sconfig option against changing the active security policy.
>>>>
>>>> I believe now a single option is too limiting, it should instead be
>>>> a directed graph that explains which policies are reachable from a given
>>>> policy.
>>>>
>>>> Anyways, the change here invalidates the Kconfig help text for
>>>> SECURITY_POLICY_INIT.
>>>>
>>>> I am not fully sure if this change is a good idea, but it needs to
>>>> be fixed to be considered. I assume you do this, because checking
>>>> the name of the policy doesn't trigger a selection like IS_ALLOWED does?
>>>
>>> exactly.
>>> during device_probe, there is a need to know the current policy name, if there is a policy active.
>>>
>>> I will have a look into it.
>>>
>>> Fabian
>>>
>>>>
>>>> Thanks,
>>>> Ahmad
>>>>
>>>>
>>>>> return 0;
>>>>> }
>>>>> pure_initcall(security_init);
>>>>>
>>>>
>>>
>>
>
--
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 |
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 4/5] security: configure pinctrl based on policy name
2026-03-18 9:21 [PATCH v3 0/5] Add helper for security policies Fabian Pflug
` (2 preceding siblings ...)
2026-03-18 9:22 ` [PATCH v3 3/5] security: policy: set active policy on boot Fabian Pflug
@ 2026-03-18 9:22 ` Fabian Pflug
2026-03-18 11:43 ` Ahmad Fatoum
2026-03-18 9:22 ` [PATCH v3 5/5] security: kernel_pinctrl: fixup pinctrl in kernel dts Fabian Pflug
2026-03-18 9:57 ` [PATCH v3 0/5] Add helper for security policies Sascha Hauer
5 siblings, 1 reply; 16+ messages in thread
From: Fabian Pflug @ 2026-03-18 9:22 UTC (permalink / raw)
To: BAREBOX, Sascha Hauer; +Cc: Fabian Pflug
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.
A Kconfig option to enable this feature has been chosen, because parsing
pinctrl and mapping the names is a lot of string operations, that could
increase boottime for a feature, that is maybe not needed for everyone.
Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
drivers/base/driver.c | 12 +++++++++++-
security/Kconfig.policy | 8 ++++++++
security/policy.c | 12 ++++++++++++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 20beb1e9e6..147c3cbad8 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -30,6 +30,7 @@
#include <pinctrl.h>
#include <featctrl.h>
#include <linux/clk/clk-conf.h>
+#include <security/policy.h>
#ifdef CONFIG_DEBUG_PROBES
#define pr_report_probe pr_info
@@ -135,7 +136,16 @@ int device_probe(struct device *dev)
pr_report_probe("%*sprobe-> %s\n", depth * 4, "", dev_name(dev));
- 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);
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.
+
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);
+ }
+
for (int i = 0; i < SCONFIG_NUM; i++) {
if (__is_allowed(policy, i) == __is_allowed(old_policy, i))
continue;
--
2.47.3
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 4/5] security: configure pinctrl based on policy name
2026-03-18 9:22 ` [PATCH v3 4/5] security: configure pinctrl based on policy name Fabian Pflug
@ 2026-03-18 11:43 ` Ahmad Fatoum
0 siblings, 0 replies; 16+ messages in thread
From: Ahmad Fatoum @ 2026-03-18 11:43 UTC (permalink / raw)
To: Fabian Pflug, BAREBOX, Sascha Hauer
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 |
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 5/5] security: kernel_pinctrl: fixup pinctrl in kernel dts
2026-03-18 9:21 [PATCH v3 0/5] Add helper for security policies Fabian Pflug
` (3 preceding siblings ...)
2026-03-18 9:22 ` [PATCH v3 4/5] security: configure pinctrl based on policy name Fabian Pflug
@ 2026-03-18 9:22 ` Fabian Pflug
2026-03-18 11:53 ` Ahmad Fatoum
2026-03-18 9:57 ` [PATCH v3 0/5] Add helper for security policies Sascha Hauer
5 siblings, 1 reply; 16+ messages in thread
From: Fabian Pflug @ 2026-03-18 9:22 UTC (permalink / raw)
To: BAREBOX, Sascha Hauer; +Cc: Fabian Pflug
Going through the kernel dts and replacing
barebox,policy-<active_policy> with default in order to change pinctrl
not only for barebox, but also for kernel when booting with security
profiles.
Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
security/Makefile | 1 +
security/kernel_pinctrl.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/security/Makefile b/security/Makefile
index 1096cbfb9b..2e8cdfe7c2 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -2,6 +2,7 @@
obj-$(CONFIG_SECURITY_POLICY) += policy.o
obj-$(CONFIG_SECURITY_POLICY_NAMES) += sconfig_names.o
+obj-$(CONFIG_SECURITY_POLICY_PINCTRL) += kernel_pinctrl.o
obj-$(CONFIG_CRYPTO_KEYSTORE) += keystore.o
obj-$(CONFIG_JWT) += jwt.o
obj-pbl-$(CONFIG_HAVE_OPTEE) += optee.o
diff --git a/security/kernel_pinctrl.c b/security/kernel_pinctrl.c
new file mode 100644
index 0000000000..01ccac3ec3
--- /dev/null
+++ b/security/kernel_pinctrl.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <common.h>
+#include <linux/printk.h>
+#include <pinctrl.h>
+#include <security/policy.h>
+#include <security/config.h>
+
+/* Maximun number of names to be available in pinctrl-names. Best guess.*/
+#define MAX_NAMES 10
+
+
+/**
+ * Replace 'default' with 'old_default' and 'barebox,policy-<active_policy>'
+ * with 'default', if both are found in pinctrl.
+ */
+static void kernel_of_fixup_pinctrl(struct device_node *root_node, char *policy_name)
+{
+ struct device_node *node;
+
+ list_for_each_entry(node, &root_node->list, list) {
+ const char *names[MAX_NAMES];
+ int num_read, pos_default = -1, pos_policy = -1;
+
+ num_read = of_property_read_string_array(node, "pinctrl-names", names, MAX_NAMES);
+
+ for (int i = 0; i < num_read; i++) {
+ if (strcmp(policy_name, names[i]) == 0)
+ pos_policy = i;
+ if (strcmp("default", names[i]) == 0)
+ pos_default = i;
+ }
+
+ if (pos_default >= 0 && pos_policy >= 0) {
+ names[pos_default] = "old_default";
+ names[pos_policy] = "default";
+ of_property_write_string_array(node, "pinctrl-names", names, num_read);
+ }
+ }
+}
+
+static int kernel_of_fixup_pinctrl_start(struct device_node *root, void *unused)
+{
+ char *policy_pinctrl;
+
+ policy_pinctrl = basprintf("barebox,policy-%s", active_policy->name);
+ kernel_of_fixup_pinctrl(root, policy_pinctrl);
+ free(policy_pinctrl);
+ return 0;
+}
+
+static int policy_console_pinctrl_init(void)
+{
+ return of_register_fixup(kernel_of_fixup_pinctrl_start, NULL);
+}
+late_initcall(policy_console_pinctrl_init);
--
2.47.3
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 5/5] security: kernel_pinctrl: fixup pinctrl in kernel dts
2026-03-18 9:22 ` [PATCH v3 5/5] security: kernel_pinctrl: fixup pinctrl in kernel dts Fabian Pflug
@ 2026-03-18 11:53 ` Ahmad Fatoum
0 siblings, 0 replies; 16+ messages in thread
From: Ahmad Fatoum @ 2026-03-18 11:53 UTC (permalink / raw)
To: Fabian Pflug, BAREBOX, Sascha Hauer
Hello,
On 3/18/26 10:22, Fabian Pflug wrote:
> Going through the kernel dts and replacing
> barebox,policy-<active_policy> with default in order to change pinctrl
> not only for barebox, but also for kernel when booting with security
> profiles.
>
> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
> ---
> security/Makefile | 1 +
> security/kernel_pinctrl.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 57 insertions(+)
>
> diff --git a/security/Makefile b/security/Makefile
> index 1096cbfb9b..2e8cdfe7c2 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -2,6 +2,7 @@
>
> obj-$(CONFIG_SECURITY_POLICY) += policy.o
> obj-$(CONFIG_SECURITY_POLICY_NAMES) += sconfig_names.o
> +obj-$(CONFIG_SECURITY_POLICY_PINCTRL) += kernel_pinctrl.o
> obj-$(CONFIG_CRYPTO_KEYSTORE) += keystore.o
> obj-$(CONFIG_JWT) += jwt.o
> obj-pbl-$(CONFIG_HAVE_OPTEE) += optee.o
> diff --git a/security/kernel_pinctrl.c b/security/kernel_pinctrl.c
> new file mode 100644
> index 0000000000..01ccac3ec3
> --- /dev/null
> +++ b/security/kernel_pinctrl.c
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <common.h>
> +#include <linux/printk.h>
> +#include <pinctrl.h>
> +#include <security/policy.h>
> +#include <security/config.h>
> +
> +/* Maximun number of names to be available in pinctrl-names. Best guess.*/
> +#define MAX_NAMES 10
> +
> +
> +/**
> + * Replace 'default' with 'old_default' and 'barebox,policy-<active_policy>'
> + * with 'default', if both are found in pinctrl.
> + */
> +static void kernel_of_fixup_pinctrl(struct device_node *root_node, char *policy_name)
> +{
> + struct device_node *node;
> +
> + list_for_each_entry(node, &root_node->list, list) {
I am surprised of.h didn't have a macro for that.
> + const char *names[MAX_NAMES];
> + int num_read, pos_default = -1, pos_policy = -1;
> +
> + num_read = of_property_read_string_array(node, "pinctrl-names", names, MAX_NAMES);
> +
> + for (int i = 0; i < num_read; i++) {
> + if (strcmp(policy_name, names[i]) == 0)
> + pos_policy = i;
> + if (strcmp("default", names[i]) == 0)
> + pos_default = i;
> + }
> +
> + if (pos_default >= 0 && pos_policy >= 0) {
> + names[pos_default] = "old_default";
> + names[pos_policy] = "default";
> + of_property_write_string_array(node, "pinctrl-names", names, num_read);
> + }
Wouldn't it be simpler to use of_property_for_each_string()?
> + }
> +}
> +
> +static int kernel_of_fixup_pinctrl_start(struct device_node *root, void *unused)
> +{
> + char *policy_pinctrl;
> +
> + policy_pinctrl = basprintf("barebox,policy-%s", active_policy->name);
> + kernel_of_fixup_pinctrl(root, policy_pinctrl);
> + free(policy_pinctrl);
> + return 0;
> +}
What's the point of this intermediary function?
> +
> +static int policy_console_pinctrl_init(void)
> +{
> + return of_register_fixup(kernel_of_fixup_pinctrl_start, NULL);
> +}
> +late_initcall(policy_console_pinctrl_init);
>
--
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 |
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 0/5] Add helper for security policies
2026-03-18 9:21 [PATCH v3 0/5] Add helper for security policies Fabian Pflug
` (4 preceding siblings ...)
2026-03-18 9:22 ` [PATCH v3 5/5] security: kernel_pinctrl: fixup pinctrl in kernel dts Fabian Pflug
@ 2026-03-18 9:57 ` Sascha Hauer
2026-03-18 11:43 ` Ahmad Fatoum
5 siblings, 1 reply; 16+ messages in thread
From: Sascha Hauer @ 2026-03-18 9:57 UTC (permalink / raw)
To: BAREBOX, Fabian Pflug
On Wed, 18 Mar 2026 10:21:58 +0100, Fabian Pflug wrote:
> This series adds helper functions to the security policy framework to
> do additional work based on the selected policy.
> Like adding the policy name to the commandline and configuring pinmux
> based on the selected policy.
>
>
Applied, thanks!
[1/5] of: add of_property_write_string_array()
https://git.pengutronix.de/cgit/barebox/commit/?id=a8ecb979939e (link may not be stable)
[2/5] common: bootm: add policy to commandline
https://git.pengutronix.de/cgit/barebox/commit/?id=d89e470bf141 (link may not be stable)
[3/5] security: policy: set active policy on boot
https://git.pengutronix.de/cgit/barebox/commit/?id=6723f82f4041 (link may not be stable)
[4/5] security: configure pinctrl based on policy name
https://git.pengutronix.de/cgit/barebox/commit/?id=821e5618a05b (link may not be stable)
[5/5] security: kernel_pinctrl: fixup pinctrl in kernel dts
https://git.pengutronix.de/cgit/barebox/commit/?id=c5ad4d3f6fcf (link may not be stable)
Best regards,
--
Sascha Hauer <s.hauer@pengutronix.de>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 0/5] Add helper for security policies
2026-03-18 9:57 ` [PATCH v3 0/5] Add helper for security policies Sascha Hauer
@ 2026-03-18 11:43 ` Ahmad Fatoum
0 siblings, 0 replies; 16+ messages in thread
From: Ahmad Fatoum @ 2026-03-18 11:43 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX, Fabian Pflug
Hello Sascha,
On 3/18/26 10:57, Sascha Hauer wrote:
>
> On Wed, 18 Mar 2026 10:21:58 +0100, Fabian Pflug wrote:
>> This series adds helper functions to the security policy framework to
>> do additional work based on the selected policy.
>> Like adding the policy name to the commandline and configuring pinmux
>> based on the selected policy.
>>
>>
>
> Applied, thanks!
>
> [1/5] of: add of_property_write_string_array()
> https://git.pengutronix.de/cgit/barebox/commit/?id=a8ecb979939e (link may not be stable)
> [2/5] common: bootm: add policy to commandline
> https://git.pengutronix.de/cgit/barebox/commit/?id=d89e470bf141 (link may not be stable)
> [3/5] security: policy: set active policy on boot
> https://git.pengutronix.de/cgit/barebox/commit/?id=6723f82f4041 (link may not be stable)
> [4/5] security: configure pinctrl based on policy name
> https://git.pengutronix.de/cgit/barebox/commit/?id=821e5618a05b (link may not be stable)
> [5/5] security: kernel_pinctrl: fixup pinctrl in kernel dts
> https://git.pengutronix.de/cgit/barebox/commit/?id=c5ad4d3f6fcf (link may not be stable)
Please revert. The series needs rework and introduces a regression AFAICS.
Thanks,
Ahmad
>
> Best regards,
--
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 |
^ permalink raw reply [flat|nested] 16+ messages in thread