mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [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

* [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 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 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

* 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 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 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: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 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