mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v2 1/3] startup: check for console before showing menu
@ 2025-10-28 15:43 Fabian Pflug
  2025-10-28 15:43 ` [PATCH v2 2/3] console_common: get_first_active: respect security policy Fabian Pflug
  2025-10-28 15:43 ` [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL Fabian Pflug
  0 siblings, 2 replies; 6+ 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] 6+ messages in thread

* [PATCH v2 2/3] console_common: get_first_active: respect security policy
  2025-10-28 15:43 [PATCH v2 1/3] startup: check for console before showing menu Fabian Pflug
@ 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
  1 sibling, 0 replies; 6+ messages in thread
From: Fabian Pflug @ 2025-10-28 15:43 UTC (permalink / raw)
  To: barebox; +Cc: Fabian Pflug

If the console input is deactivated through a security policy, then
there is no need to iterate over the current consoles, as none should
have a STDIN.

Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
v2:
Fix in console common instead of working around it in startup.c
 common/console_common.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/common/console_common.c b/common/console_common.c
index 5b7a64c99c..a8319c20f3 100644
--- a/common/console_common.c
+++ b/common/console_common.c
@@ -23,6 +23,7 @@
 #include <linux/math64.h>
 #include <linux/sizes.h>
 #include <linux/overflow.h>
+#include <security/config.h>
 
 #ifndef CONFIG_CONSOLE_NONE
 
@@ -331,6 +332,11 @@ struct console_device *console_get_first_active(void)
 {
 	struct console_device *cdev;
 	const unsigned char active = CONSOLE_STDIN | CONSOLE_STDOUT;
+
+	/* if no console input is allows, then we can't have STDIN on any. */
+	if (!IS_ALLOWED(SCONFIG_CONSOLE_INPUT))
+		return NULL;
+
 	/*
 	 * Assumption to have BOTH CONSOLE_STDIN AND STDOUT in the
 	 * same output console
-- 
2.47.3




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL
  2025-10-28 15:43 [PATCH v2 1/3] startup: check for console before showing menu Fabian Pflug
  2025-10-28 15:43 ` [PATCH v2 2/3] console_common: get_first_active: respect security policy Fabian Pflug
@ 2025-10-28 15:43 ` Fabian Pflug
  1 sibling, 0 replies; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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 ` Fabian Pflug
  2025-10-28 12:56   ` Ahmad Fatoum
  0 siblings, 1 reply; 6+ 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] 6+ messages in thread

end of thread, other threads:[~2025-10-28 15:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-28 15:43 [PATCH v2 1/3] startup: check for console before showing menu Fabian Pflug
2025-10-28 15:43 ` [PATCH v2 2/3] console_common: get_first_active: respect security policy Fabian Pflug
2025-10-28 15:43 ` [PATCH 3/3] startup: mount ps only on policy FS_EXTERNAL Fabian Pflug
  -- strict thread matches above, loose matches on Subject: below --
2025-10-28 12:18 [PATCH 1/3] startup: check for console before showing menu Fabian Pflug
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox