* [PATCH 1/3] startup: check for console before showing menu
@ 2025-10-28 12:18 Fabian Pflug
2025-10-28 12:18 ` [PATCH 2/3] startup: autoboot: disable on deactivated console Fabian Pflug
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Fabian Pflug @ 2025-10-28 12:18 UTC (permalink / raw)
To: barebox; +Cc: Fabian Pflug
If there is no input available or possible due to policy settings, it
does not make sense to show a menu and ask for input.
Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
common/startup.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/common/startup.c b/common/startup.c
index 8d36ffceb4..3bc2609006 100644
--- a/common/startup.c
+++ b/common/startup.c
@@ -45,6 +45,7 @@
#include <pbl/handoff-data.h>
#include <libfile.h>
#include <fuzz.h>
+#include <security/config.h>
extern initcall_t __barebox_initcalls_start[], __barebox_early_initcalls_end[],
__barebox_initcalls_end[];
@@ -361,14 +362,15 @@ static int run_init(void)
run_shell();
}
- do {
- /*
- * Let's run the command once at least, so an error
- * message is printed if the file doesn't exist
- */
- run_command(MENUFILE);
- } while (stat(MENUFILE, &s) == 0);
-
+ if(!IS_ENABLED(CONFIG_CONSOLE_DISABLE_INPUT) && IS_ALLOWED(SCONFIG_CONSOLE_INPUT)) {
+ do {
+ /*
+ * Let's run the command once at least, so an error
+ * message is printed if the file doesn't exist
+ */
+ run_command(MENUFILE);
+ } while (stat(MENUFILE, &s) == 0);
+ }
hang();
}
--
2.47.3
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 2/3] startup: autoboot: disable on deactivated console 2025-10-28 12:18 [PATCH 1/3] startup: check for console before showing menu Fabian Pflug @ 2025-10-28 12:18 ` Fabian Pflug 2025-10-28 12:49 ` Ahmad Fatoum 2025-10-28 12:18 ` [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL Fabian Pflug 2025-10-28 12:39 ` [PATCH 1/3] startup: check for console before showing menu Ahmad Fatoum 2 siblings, 1 reply; 11+ messages in thread From: Fabian Pflug @ 2025-10-28 12:18 UTC (permalink / raw) To: barebox; +Cc: Fabian Pflug If the console input is deactivated either globally or through a security policy, then it does not make sense to wait for user input, but instead boot the system directly. Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de> --- common/startup.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/common/startup.c b/common/startup.c index 3bc2609006..ea5436afa6 100644 --- a/common/startup.c +++ b/common/startup.c @@ -188,9 +188,11 @@ enum autoboot_state do_autoboot_countdown(void) if (autoboot_state != AUTOBOOT_UNKNOWN) return autoboot_state; - if (!console_get_first_active() && - global_autoboot_state != AUTOBOOT_ABORT && - global_autoboot_state != AUTOBOOT_HALT) { + if ((!console_get_first_active() && + global_autoboot_state != AUTOBOOT_ABORT && + global_autoboot_state != AUTOBOOT_HALT) || + !IS_ALLOWED(SCONFIG_CONSOLE_INPUT) || + IS_ENABLED(CONFIG_CONSOLE_DISABLE_INPUT)) { printf("\nNon-interactive console, booting system\n"); return autoboot_state = AUTOBOOT_BOOT; } -- 2.47.3 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] startup: autoboot: disable on deactivated console 2025-10-28 12:18 ` [PATCH 2/3] startup: autoboot: disable on deactivated console Fabian Pflug @ 2025-10-28 12:49 ` Ahmad Fatoum 0 siblings, 0 replies; 11+ messages in thread From: Ahmad Fatoum @ 2025-10-28 12:49 UTC (permalink / raw) To: Fabian Pflug, barebox On 10/28/25 1:18 PM, Fabian Pflug wrote: > If the console input is deactivated either globally or through a > security policy, then it does not make sense to wait for user input, but > instead boot the system directly. CONFIG_CONSOLE_DISABLE_INPUT is unneeded here as it removed the input flag from the consoles and so console_get_first_active would return NULL. I think console_get_first_active() should maybe return NULL if SCONFIG_CONSOLE_INPUT is disabled. In that case, we should probably rename it, e.g. console_get_first_interactive. > > Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de> > --- > common/startup.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/common/startup.c b/common/startup.c > index 3bc2609006..ea5436afa6 100644 > --- a/common/startup.c > +++ b/common/startup.c > @@ -188,9 +188,11 @@ enum autoboot_state do_autoboot_countdown(void) > if (autoboot_state != AUTOBOOT_UNKNOWN) > return autoboot_state; > > - if (!console_get_first_active() && > - global_autoboot_state != AUTOBOOT_ABORT && > - global_autoboot_state != AUTOBOOT_HALT) { > + if ((!console_get_first_active() && > + global_autoboot_state != AUTOBOOT_ABORT && > + global_autoboot_state != AUTOBOOT_HALT) || > + !IS_ALLOWED(SCONFIG_CONSOLE_INPUT) || > + IS_ENABLED(CONFIG_CONSOLE_DISABLE_INPUT)) { > printf("\nNon-interactive console, booting system\n"); > return autoboot_state = AUTOBOOT_BOOT; > } -- 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] 11+ messages in thread
* [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL 2025-10-28 12:18 [PATCH 1/3] startup: check for console before showing menu Fabian Pflug 2025-10-28 12:18 ` [PATCH 2/3] startup: autoboot: disable on deactivated console Fabian Pflug @ 2025-10-28 12:18 ` Fabian Pflug 2025-10-28 12:56 ` Ahmad Fatoum 2025-10-28 12:39 ` [PATCH 1/3] startup: check for console before showing menu Ahmad Fatoum 2 siblings, 1 reply; 11+ messages in thread From: Fabian Pflug @ 2025-10-28 12:18 UTC (permalink / raw) To: barebox; +Cc: Fabian Pflug Without the SCONFIG_FS_EXTERNAL, the bus of the driver for pstore will not load, resulting in a missing driver for pstore and an error during bootup. Only mount the /pstore if FS_EXTERNAL is allowed by the security policy. Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de> --- common/startup.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/common/startup.c b/common/startup.c index ea5436afa6..f16a99f7e4 100644 --- a/common/startup.c +++ b/common/startup.c @@ -55,6 +55,19 @@ extern exitcall_t __barebox_exitcalls_start[], __barebox_exitcalls_end[]; enum system_states barebox_system_state; #if defined CONFIG_FS_RAMFS && defined CONFIG_FS_DEVFS +static struct sconfig_notifier_block sconfig_notifier; +static void u_mount_pstore(struct sconfig_notifier_block *nb, + enum security_config_option opt, bool allowed) +{ + if (allowed) { + mkdir("/pstore", 0); + mount("none", "pstore", "/pstore", NULL); + } else { + umount("/pstore"); + rmdir("/pstore"); + } +} + static int mount_root(void) { mount("none", "ramfs", "/", NULL); @@ -69,8 +82,13 @@ static int mount_root(void) } if (IS_ENABLED(CONFIG_FS_PSTORE)) { - mkdir("/pstore", 0); - mount("none", "pstore", "/pstore", NULL); + if (IS_ALLOWED(SCONFIG_FS_EXTERNAL)) { + mkdir("/pstore", 0); + mount("none", "pstore", "/pstore", NULL); + } + sconfig_register_handler_filtered(&sconfig_notifier, + u_mount_pstore, + SCONFIG_FS_EXTERNAL); } if (IS_ENABLED(CONFIG_9P_FS)) -- 2.47.3 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL 2025-10-28 12:18 ` [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL Fabian Pflug @ 2025-10-28 12:56 ` Ahmad Fatoum 2025-10-28 13:48 ` Fabian Pflug 0 siblings, 1 reply; 11+ messages in thread From: Ahmad Fatoum @ 2025-10-28 12:56 UTC (permalink / raw) To: Fabian Pflug, barebox Hi, On 10/28/25 1:18 PM, Fabian Pflug wrote: > Without the SCONFIG_FS_EXTERNAL, the bus of the driver for pstore will > not load, resulting in a missing driver for pstore and an error during > bootup. > Only mount the /pstore if FS_EXTERNAL is allowed by the security policy. > > Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de> > --- > common/startup.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/common/startup.c b/common/startup.c > index ea5436afa6..f16a99f7e4 100644 > --- a/common/startup.c > +++ b/common/startup.c > @@ -55,6 +55,19 @@ extern exitcall_t __barebox_exitcalls_start[], __barebox_exitcalls_end[]; > enum system_states barebox_system_state; > > #if defined CONFIG_FS_RAMFS && defined CONFIG_FS_DEVFS > +static struct sconfig_notifier_block sconfig_notifier; > +static void u_mount_pstore(struct sconfig_notifier_block *nb, > + enum security_config_option opt, bool allowed) > +{ > + if (allowed) { > + mkdir("/pstore", 0); I think we should create the directory unconditionally without paying respect to whether we can mount or not. > + mount("none", "pstore", "/pstore", NULL); > + } else { > + umount("/pstore"); The harm is already done when loosening security mode, so I don't think we want to start unmounting things. As mentioned, I'd prefer replacing SCONFIG_FS_EXTERNAL altogether with a whitelist of mounts, so common code calls: allow_mount("none", "ramfs", "/", NULL); allow_mount("none", "devfs", "/dev", NULL); and board code can call: allow_mount("non", "pstore", "/pstore", NULL); and we won't need any special handling here. Waiting to see what Sascha thinks. Cheers, Ahmad > + rmdir("/pstore"); > + } > +} > + > static int mount_root(void) > { > mount("none", "ramfs", "/", NULL); > @@ -69,8 +82,13 @@ static int mount_root(void) > } > > if (IS_ENABLED(CONFIG_FS_PSTORE)) { > - mkdir("/pstore", 0); > - mount("none", "pstore", "/pstore", NULL); > + if (IS_ALLOWED(SCONFIG_FS_EXTERNAL)) { > + mkdir("/pstore", 0); > + mount("none", "pstore", "/pstore", NULL); > + } > + sconfig_register_handler_filtered(&sconfig_notifier, > + u_mount_pstore, > + SCONFIG_FS_EXTERNAL); > } > > if (IS_ENABLED(CONFIG_9P_FS)) -- 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] 11+ messages in thread
* Re: [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL 2025-10-28 12:56 ` Ahmad Fatoum @ 2025-10-28 13:48 ` Fabian Pflug 0 siblings, 0 replies; 11+ messages in thread From: Fabian Pflug @ 2025-10-28 13:48 UTC (permalink / raw) To: Ahmad Fatoum, barebox Hey :) On Tue, 2025-10-28 at 13:56 +0100, Ahmad Fatoum wrote: > Hi, > > On 10/28/25 1:18 PM, Fabian Pflug wrote: > > Without the SCONFIG_FS_EXTERNAL, the bus of the driver for pstore will > > not load, resulting in a missing driver for pstore and an error during > > bootup. > > Only mount the /pstore if FS_EXTERNAL is allowed by the security policy. > > > Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de> > > --- > > common/startup.c | 22 ++++++++++++++++++++-- > > 1 file changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/common/startup.c b/common/startup.c > > index ea5436afa6..f16a99f7e4 100644 > > --- a/common/startup.c > > +++ b/common/startup.c > > @@ -55,6 +55,19 @@ extern exitcall_t __barebox_exitcalls_start[], __barebox_exitcalls_end[]; > > enum system_states barebox_system_state; > > > > #if defined CONFIG_FS_RAMFS && defined CONFIG_FS_DEVFS > > +static struct sconfig_notifier_block sconfig_notifier; > > +static void u_mount_pstore(struct sconfig_notifier_block *nb, > > + enum security_config_option opt, bool allowed) > > +{ > > + if (allowed) { > > + mkdir("/pstore", 0); > > I think we should create the directory unconditionally without paying > respect to whether we can mount or not. I think that could irritate people into thinking, that there is nothing inside the pstore. When there is no /pstore directory, it is clear, that the mount is not there and one gets to wonder why and what happens here instead of chasing around, why the pstore is empty. > > > + mount("none", "pstore", "/pstore", NULL); > > + } else { > > + umount("/pstore"); > > The harm is already done when loosening security mode, so I don't think > we want to start unmounting things. That I can do. :) > > As mentioned, I'd prefer replacing SCONFIG_FS_EXTERNAL altogether with a > whitelist of mounts, so common code calls: > > allow_mount("none", "ramfs", "/", NULL); > allow_mount("none", "devfs", "/dev", NULL); > > and board code can call: > > allow_mount("non", "pstore", "/pstore", NULL); > > and we won't need any special handling here. Waiting to see what Sascha > thinks. I see your Idea and think this could be good, but also need something fast for me now and this works in that regard. I see yours as a long term solution that is a bit more complicated that what I know about barebox. Kind regards Fabian > > Cheers, > Ahmad > > > + rmdir("/pstore"); > > + } > > +} > > + > > static int mount_root(void) > > { > > mount("none", "ramfs", "/", NULL); > > @@ -69,8 +82,13 @@ static int mount_root(void) > > } > > > > if (IS_ENABLED(CONFIG_FS_PSTORE)) { > > - mkdir("/pstore", 0); > > - mount("none", "pstore", "/pstore", NULL); > > + if (IS_ALLOWED(SCONFIG_FS_EXTERNAL)) { > > + mkdir("/pstore", 0); > > + mount("none", "pstore", "/pstore", NULL); > > + } > > + sconfig_register_handler_filtered(&sconfig_notifier, > > + u_mount_pstore, > > + SCONFIG_FS_EXTERNAL); > > } > > > > if (IS_ENABLED(CONFIG_9P_FS)) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] startup: check for console before showing menu 2025-10-28 12:18 [PATCH 1/3] startup: check for console before showing menu Fabian Pflug 2025-10-28 12:18 ` [PATCH 2/3] startup: autoboot: disable on deactivated console Fabian Pflug 2025-10-28 12:18 ` [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL Fabian Pflug @ 2025-10-28 12:39 ` Ahmad Fatoum 2025-10-28 13:43 ` Fabian Pflug 2025-10-29 7:56 ` Sascha Hauer 2 siblings, 2 replies; 11+ messages in thread From: Ahmad Fatoum @ 2025-10-28 12:39 UTC (permalink / raw) To: Fabian Pflug, barebox Hi Fabian, On 10/28/25 1:18 PM, Fabian Pflug wrote: > If there is no input available or possible due to policy settings, it > does not make sense to show a menu and ask for input. > > Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de> > --- > common/startup.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/common/startup.c b/common/startup.c > index 8d36ffceb4..3bc2609006 100644 > --- a/common/startup.c > +++ b/common/startup.c > @@ -45,6 +45,7 @@ > #include <pbl/handoff-data.h> > #include <libfile.h> > #include <fuzz.h> > +#include <security/config.h> > > extern initcall_t __barebox_initcalls_start[], __barebox_early_initcalls_end[], > __barebox_initcalls_end[]; > @@ -361,14 +362,15 @@ static int run_init(void) > run_shell(); > } > > - do { > - /* > - * Let's run the command once at least, so an error > - * message is printed if the file doesn't exist > - */ > - run_command(MENUFILE); > - } while (stat(MENUFILE, &s) == 0); > - > + if(!IS_ENABLED(CONFIG_CONSOLE_DISABLE_INPUT) && IS_ALLOWED(SCONFIG_CONSOLE_INPUT)) { CONFIG_CONSOLE_DISABLE_INPUT only influences the default, it can still be changed later at runtime, so this is not necessarily correct. > + do { Move the IS_ALLOWED(SCONFIG_CONSOLE_INPUT) check here? Feels more natural to do it in the loop than outside. Extra benefit: We could use it for testing purposes (Menu entry that locks the console..) Cheers, Ahmad > + /* > + * Let's run the command once at least, so an error > + * message is printed if the file doesn't exist > + */ > + run_command(MENUFILE); > + } while (stat(MENUFILE, &s) == 0); > + } > hang(); > } > -- 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] 11+ messages in thread
* Re: [PATCH 1/3] startup: check for console before showing menu 2025-10-28 12:39 ` [PATCH 1/3] startup: check for console before showing menu Ahmad Fatoum @ 2025-10-28 13:43 ` Fabian Pflug 2025-10-29 7:56 ` Sascha Hauer 1 sibling, 0 replies; 11+ messages in thread From: Fabian Pflug @ 2025-10-28 13:43 UTC (permalink / raw) To: Ahmad Fatoum, barebox Hello Ahmad, On Tue, 2025-10-28 at 13:39 +0100, Ahmad Fatoum wrote: > Hi Fabian, > > On 10/28/25 1:18 PM, Fabian Pflug wrote: > > If there is no input available or possible due to policy settings, it > > does not make sense to show a menu and ask for input. > > > > Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de> > > --- > > common/startup.c | 18 ++++++++++-------- > > 1 file changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/common/startup.c b/common/startup.c > > index 8d36ffceb4..3bc2609006 100644 > > --- a/common/startup.c > > +++ b/common/startup.c > > @@ -45,6 +45,7 @@ > > #include <pbl/handoff-data.h> > > #include <libfile.h> > > #include <fuzz.h> > > +#include <security/config.h> > > > > extern initcall_t __barebox_initcalls_start[], __barebox_early_initcalls_end[], > > __barebox_initcalls_end[]; > > @@ -361,14 +362,15 @@ static int run_init(void) > > run_shell(); > > } > > > > - do { > > - /* > > - * Let's run the command once at least, so an error > > - * message is printed if the file doesn't exist > > - */ > > - run_command(MENUFILE); > > - } while (stat(MENUFILE, &s) == 0); > > - > > + if(!IS_ENABLED(CONFIG_CONSOLE_DISABLE_INPUT) && IS_ALLOWED(SCONFIG_CONSOLE_INPUT)) { > > CONFIG_CONSOLE_DISABLE_INPUT only influences the default, it can still > be changed later at runtime, so this is not necessarily correct. > > > + do { > > Move the IS_ALLOWED(SCONFIG_CONSOLE_INPUT) check here? > > Feels more natural to do it in the loop than outside. Extra benefit: We > could use it for testing purposes (Menu entry that locks the console..) I see the extra benefit for testing the console, but at this point, if there is no input possible, then we cannot enter something into the console. The menu is a new screen on the device, which makes it hard to read the possible error messages before going to console here, because the security policy did something during development, that was not intended. I would love to have the system hang here instead and tell me as much. I will not stop you in implementing it differently, but I will not do that. Kind regards Fabian > > Cheers, > Ahmad > > > + /* > > + * Let's run the command once at least, so an error > > + * message is printed if the file doesn't exist > > + */ > > + run_command(MENUFILE); > > + } while (stat(MENUFILE, &s) == 0); > > + } > > hang(); > > } > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] startup: check for console before showing menu 2025-10-28 12:39 ` [PATCH 1/3] startup: check for console before showing menu Ahmad Fatoum 2025-10-28 13:43 ` Fabian Pflug @ 2025-10-29 7:56 ` Sascha Hauer 2025-10-29 9:22 ` Sascha Hauer 1 sibling, 1 reply; 11+ messages in thread From: Sascha Hauer @ 2025-10-29 7:56 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: Fabian Pflug, barebox On Tue, Oct 28, 2025 at 01:39:22PM +0100, Ahmad Fatoum wrote: > Hi Fabian, > > On 10/28/25 1:18 PM, Fabian Pflug wrote: > > If there is no input available or possible due to policy settings, it > > does not make sense to show a menu and ask for input. > > > > Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de> > > --- > > common/startup.c | 18 ++++++++++-------- > > 1 file changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/common/startup.c b/common/startup.c > > index 8d36ffceb4..3bc2609006 100644 > > --- a/common/startup.c > > +++ b/common/startup.c > > @@ -45,6 +45,7 @@ > > #include <pbl/handoff-data.h> > > #include <libfile.h> > > #include <fuzz.h> > > +#include <security/config.h> > > > > extern initcall_t __barebox_initcalls_start[], __barebox_early_initcalls_end[], > > __barebox_initcalls_end[]; > > @@ -361,14 +362,15 @@ static int run_init(void) > > run_shell(); > > } > > > > - do { > > - /* > > - * Let's run the command once at least, so an error > > - * message is printed if the file doesn't exist > > - */ > > - run_command(MENUFILE); > > - } while (stat(MENUFILE, &s) == 0); > > - > > + if(!IS_ENABLED(CONFIG_CONSOLE_DISABLE_INPUT) && IS_ALLOWED(SCONFIG_CONSOLE_INPUT)) { > > CONFIG_CONSOLE_DISABLE_INPUT only influences the default, it can still > be changed later at runtime, so this is not necessarily correct. I agree with this. I just noticed we already have this in the code and we'll want to reconsider this: if (IS_ENABLED(CONFIG_NET) && !IS_ENABLED(CONFIG_CONSOLE_DISABLE_INPUT) && autoboot != AUTOBOOT_HALT) eth_open_all(); How about replacing the test for CONFIG_CONSOLE_DISABLE_INPUT with a runtime decision like: bool console_can_input(void) { struct console_device *cdev; if (!IS_ALLOWED(SCONFIG_CONSOLE_INPUT)) return false; for_each_console(cdev) { if (cdev->f_active & CONSOLE_STDIN) return true; } return false; } 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] 11+ messages in thread
* Re: [PATCH 1/3] startup: check for console before showing menu 2025-10-29 7:56 ` Sascha Hauer @ 2025-10-29 9:22 ` Sascha Hauer 0 siblings, 0 replies; 11+ messages in thread From: Sascha Hauer @ 2025-10-29 9:22 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: Fabian Pflug, barebox On Wed, Oct 29, 2025 at 08:56:14AM +0100, Sascha Hauer wrote: > On Tue, Oct 28, 2025 at 01:39:22PM +0100, Ahmad Fatoum wrote: > > Hi Fabian, > > > > On 10/28/25 1:18 PM, Fabian Pflug wrote: > > > If there is no input available or possible due to policy settings, it > > > does not make sense to show a menu and ask for input. > > > > > > Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de> > > > --- > > > common/startup.c | 18 ++++++++++-------- > > > 1 file changed, 10 insertions(+), 8 deletions(-) > > > > > > diff --git a/common/startup.c b/common/startup.c > > > index 8d36ffceb4..3bc2609006 100644 > > > --- a/common/startup.c > > > +++ b/common/startup.c > > > @@ -45,6 +45,7 @@ > > > #include <pbl/handoff-data.h> > > > #include <libfile.h> > > > #include <fuzz.h> > > > +#include <security/config.h> > > > > > > extern initcall_t __barebox_initcalls_start[], __barebox_early_initcalls_end[], > > > __barebox_initcalls_end[]; > > > @@ -361,14 +362,15 @@ static int run_init(void) > > > run_shell(); > > > } > > > > > > - do { > > > - /* > > > - * Let's run the command once at least, so an error > > > - * message is printed if the file doesn't exist > > > - */ > > > - run_command(MENUFILE); > > > - } while (stat(MENUFILE, &s) == 0); > > > - > > > + if(!IS_ENABLED(CONFIG_CONSOLE_DISABLE_INPUT) && IS_ALLOWED(SCONFIG_CONSOLE_INPUT)) { > > > > CONFIG_CONSOLE_DISABLE_INPUT only influences the default, it can still > > be changed later at runtime, so this is not necessarily correct. > > I agree with this. I just noticed we already have this in the code > and we'll want to reconsider this: > > if (IS_ENABLED(CONFIG_NET) && !IS_ENABLED(CONFIG_CONSOLE_DISABLE_INPUT) && > autoboot != AUTOBOOT_HALT) > eth_open_all(); > > How about replacing the test for CONFIG_CONSOLE_DISABLE_INPUT with a > runtime decision like: > > bool console_can_input(void) > { > struct console_device *cdev; > > if (!IS_ALLOWED(SCONFIG_CONSOLE_INPUT)) > return false; > > for_each_console(cdev) { > if (cdev->f_active & CONSOLE_STDIN) > return true; > } > > return false; > } Ok, your patch with extending console_get_first_active() effectively does this, so disregard this comment. 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] 11+ messages in thread
* [PATCH v2 1/3] startup: check for console before showing menu @ 2025-10-28 15:43 Fabian Pflug 2025-10-28 15:43 ` [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL Fabian Pflug 0 siblings, 1 reply; 11+ messages in thread From: Fabian Pflug @ 2025-10-28 15:43 UTC (permalink / raw) To: barebox; +Cc: Fabian Pflug If there is no input available or possible due to policy settings, it does not make sense to show a menu and ask for input. Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de> --- v2: Remove CONSOLE_DISABLE_INPUT from check common/startup.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/common/startup.c b/common/startup.c index 8d36ffceb4..4313435f05 100644 --- a/common/startup.c +++ b/common/startup.c @@ -45,6 +45,7 @@ #include <pbl/handoff-data.h> #include <libfile.h> #include <fuzz.h> +#include <security/config.h> extern initcall_t __barebox_initcalls_start[], __barebox_early_initcalls_end[], __barebox_initcalls_end[]; @@ -361,14 +362,15 @@ static int run_init(void) run_shell(); } - do { - /* - * Let's run the command once at least, so an error - * message is printed if the file doesn't exist - */ - run_command(MENUFILE); - } while (stat(MENUFILE, &s) == 0); - + if(IS_ALLOWED(SCONFIG_CONSOLE_INPUT)) { + do { + /* + * Let's run the command once at least, so an error + * message is printed if the file doesn't exist + */ + run_command(MENUFILE); + } while (stat(MENUFILE, &s) == 0); + } hang(); } -- 2.47.3 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL 2025-10-28 15:43 [PATCH v2 " Fabian Pflug @ 2025-10-28 15:43 ` Fabian Pflug 0 siblings, 0 replies; 11+ messages in thread From: Fabian Pflug @ 2025-10-28 15:43 UTC (permalink / raw) To: barebox; +Cc: Fabian Pflug Without the SCONFIG_FS_EXTERNAL, the bus of the driver for pstore will not load, resulting in a missing driver for pstore and an error during bootup. Only mount the /pstore if FS_EXTERNAL is allowed by the security policy. Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de> --- v2: remove the umount add unregister_handler common/startup.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/common/startup.c b/common/startup.c index 4313435f05..7cf7088ad6 100644 --- a/common/startup.c +++ b/common/startup.c @@ -55,6 +55,22 @@ extern exitcall_t __barebox_exitcalls_start[], __barebox_exitcalls_end[]; enum system_states barebox_system_state; #if defined CONFIG_FS_RAMFS && defined CONFIG_FS_DEVFS +static struct sconfig_notifier_block sconfig_notifier; + +static void mount_pstore(struct sconfig_notifier_block *nb, + enum security_config_option opt, bool allowed) +{ + if (allowed) { + mkdir("/pstore", 0); + mount("none", "pstore", "/pstore", NULL); + sconfig_unregister_handler(&sconfig_notifier); + } + /* + * no need to umount, since the permission is only needed for mounting, + * not for accessing the content. + */ +} + static int mount_root(void) { mount("none", "ramfs", "/", NULL); @@ -69,8 +85,14 @@ static int mount_root(void) } if (IS_ENABLED(CONFIG_FS_PSTORE)) { - mkdir("/pstore", 0); - mount("none", "pstore", "/pstore", NULL); + if (IS_ALLOWED(SCONFIG_FS_EXTERNAL)) { + mkdir("/pstore", 0); + mount("none", "pstore", "/pstore", NULL); + } else { + sconfig_register_handler_filtered(&sconfig_notifier, + mount_pstore, + SCONFIG_FS_EXTERNAL); + } } if (IS_ENABLED(CONFIG_9P_FS)) -- 2.47.3 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-10-29 9:23 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-10-28 12:18 [PATCH 1/3] startup: check for console before showing menu Fabian Pflug 2025-10-28 12:18 ` [PATCH 2/3] startup: autoboot: disable on deactivated console Fabian Pflug 2025-10-28 12:49 ` Ahmad Fatoum 2025-10-28 12:18 ` [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL Fabian Pflug 2025-10-28 12:56 ` Ahmad Fatoum 2025-10-28 13:48 ` Fabian Pflug 2025-10-28 12:39 ` [PATCH 1/3] startup: check for console before showing menu Ahmad Fatoum 2025-10-28 13:43 ` Fabian Pflug 2025-10-29 7:56 ` Sascha Hauer 2025-10-29 9:22 ` Sascha Hauer 2025-10-28 15:43 [PATCH v2 " Fabian Pflug 2025-10-28 15:43 ` [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL Fabian Pflug
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox