* [PATCH 0/2] env: Enhancements for establishing variable protection @ 2023-02-03 12:22 Jan Kiszka 2023-02-03 12:22 ` [PATCH 1/2] env: Complete generic support for writable list Jan Kiszka 2023-02-03 12:22 ` [PATCH 2/2] env: Couple networking-related variable flags to CONFIG_NET Jan Kiszka 0 siblings, 2 replies; 9+ messages in thread From: Jan Kiszka @ 2023-02-03 12:22 UTC (permalink / raw) To: U-Boot Mailing List; +Cc: Joe Hershberger, Marek Vasut, Stefan Herbrechtsmeier This was factored out of [1] upon request in the hope of easing the merge. Jan [1] https://lore.kernel.org/u-boot/cover.1675325279.git.jan.kiszka@siemens.com CC: Joe Hershberger <joe.hershberger@ni.com> CC: Marek Vasut <marex@denx.de> CC: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com> Jan Kiszka (2): env: Complete generic support for writable list env: Couple networking-related variable flags to CONFIG_NET env/Kconfig | 1 + env/env.c | 8 ++++++++ env/flags.c | 10 +++++----- include/env_flags.h | 4 ++-- 4 files changed, 16 insertions(+), 7 deletions(-) -- 2.35.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] env: Complete generic support for writable list 2023-02-03 12:22 [PATCH 0/2] env: Enhancements for establishing variable protection Jan Kiszka @ 2023-02-03 12:22 ` Jan Kiszka 2023-02-07 4:02 ` Simon Glass 2023-02-10 18:44 ` Tom Rini 2023-02-03 12:22 ` [PATCH 2/2] env: Couple networking-related variable flags to CONFIG_NET Jan Kiszka 1 sibling, 2 replies; 9+ messages in thread From: Jan Kiszka @ 2023-02-03 12:22 UTC (permalink / raw) To: U-Boot Mailing List; +Cc: Joe Hershberger, Marek Vasut, Stefan Herbrechtsmeier From: Jan Kiszka <jan.kiszka@siemens.com> This completes what 890feecaab72 started by selecting ENV_APPEND and loading the default env before any other sources. This ensures that load operations pick up all non-writable vars from the default env and only permitted parts from other locations according to the regular priorities. With this change, boards only need to define the list of writable variables but no longer have to provide a custom env_get_location implementation. CC: Joe Hershberger <joe.hershberger@ni.com> CC: Marek Vasut <marex@denx.de> CC: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Reviewed-by: Marek Vasut <marex@denx.de> --- env/Kconfig | 1 + env/env.c | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/env/Kconfig b/env/Kconfig index c409ea71fe5..6e24eee55f2 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -733,6 +733,7 @@ config ENV_APPEND config ENV_WRITEABLE_LIST bool "Permit write access only to listed variables" + select ENV_APPEND help If defined, only environment variables which explicitly set the 'w' writeable flag can be written and modified at runtime. No variables diff --git a/env/env.c b/env/env.c index 06078c7f374..45e638fcd1f 100644 --- a/env/env.c +++ b/env/env.c @@ -195,6 +195,14 @@ int env_load(void) int best_prio = -1; int prio; + if (CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)) { + /* + * When using a list of writeable variables, the baseline comes + * from the built-in default env. So load this first. + */ + env_set_default(NULL, 0); + } + for (prio = 0; (drv = env_driver_lookup(ENVOP_LOAD, prio)); prio++) { int ret; -- 2.35.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] env: Complete generic support for writable list 2023-02-03 12:22 ` [PATCH 1/2] env: Complete generic support for writable list Jan Kiszka @ 2023-02-07 4:02 ` Simon Glass 2023-02-07 5:49 ` Jan Kiszka 2023-02-10 18:44 ` Tom Rini 1 sibling, 1 reply; 9+ messages in thread From: Simon Glass @ 2023-02-07 4:02 UTC (permalink / raw) To: Jan Kiszka Cc: U-Boot Mailing List, Joe Hershberger, Marek Vasut, Stefan Herbrechtsmeier Hi Jan, On Fri, 3 Feb 2023 at 05:23, Jan Kiszka <jan.kiszka@siemens.com> wrote: > > From: Jan Kiszka <jan.kiszka@siemens.com> > > This completes what 890feecaab72 started by selecting ENV_APPEND and > loading the default env before any other sources. This ensures that load > operations pick up all non-writable vars from the default env and only > permitted parts from other locations according to the regular > priorities. > > With this change, boards only need to define the list of writable > variables but no longer have to provide a custom env_get_location > implementation. > > CC: Joe Hershberger <joe.hershberger@ni.com> > CC: Marek Vasut <marex@denx.de> > CC: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > Reviewed-by: Marek Vasut <marex@denx.de> > --- > env/Kconfig | 1 + > env/env.c | 8 ++++++++ > 2 files changed, 9 insertions(+) > > diff --git a/env/Kconfig b/env/Kconfig > index c409ea71fe5..6e24eee55f2 100644 > --- a/env/Kconfig > +++ b/env/Kconfig > @@ -733,6 +733,7 @@ config ENV_APPEND > > config ENV_WRITEABLE_LIST > bool "Permit write access only to listed variables" > + select ENV_APPEND > help > If defined, only environment variables which explicitly set the 'w' > writeable flag can be written and modified at runtime. No variables > diff --git a/env/env.c b/env/env.c > index 06078c7f374..45e638fcd1f 100644 > --- a/env/env.c > +++ b/env/env.c > @@ -195,6 +195,14 @@ int env_load(void) > int best_prio = -1; > int prio; > > + if (CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)) { > + /* > + * When using a list of writeable variables, the baseline comes > + * from the built-in default env. So load this first. > + */ > + env_set_default(NULL, 0); > + } > + > for (prio = 0; (drv = env_driver_lookup(ENVOP_LOAD, prio)); prio++) { > int ret; > > -- > 2.35.3 > This looks OK, but please can you write some tests in test/env ? Regards, SImon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] env: Complete generic support for writable list 2023-02-07 4:02 ` Simon Glass @ 2023-02-07 5:49 ` Jan Kiszka 2023-02-07 13:38 ` Simon Glass 0 siblings, 1 reply; 9+ messages in thread From: Jan Kiszka @ 2023-02-07 5:49 UTC (permalink / raw) To: Simon Glass Cc: U-Boot Mailing List, Joe Hershberger, Marek Vasut, Stefan Herbrechtsmeier On 07.02.23 05:02, Simon Glass wrote: > Hi Jan, > > On Fri, 3 Feb 2023 at 05:23, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> This completes what 890feecaab72 started by selecting ENV_APPEND and >> loading the default env before any other sources. This ensures that load >> operations pick up all non-writable vars from the default env and only >> permitted parts from other locations according to the regular >> priorities. >> >> With this change, boards only need to define the list of writable >> variables but no longer have to provide a custom env_get_location >> implementation. >> >> CC: Joe Hershberger <joe.hershberger@ni.com> >> CC: Marek Vasut <marex@denx.de> >> CC: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> Reviewed-by: Marek Vasut <marex@denx.de> >> --- >> env/Kconfig | 1 + >> env/env.c | 8 ++++++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/env/Kconfig b/env/Kconfig >> index c409ea71fe5..6e24eee55f2 100644 >> --- a/env/Kconfig >> +++ b/env/Kconfig >> @@ -733,6 +733,7 @@ config ENV_APPEND >> >> config ENV_WRITEABLE_LIST >> bool "Permit write access only to listed variables" >> + select ENV_APPEND >> help >> If defined, only environment variables which explicitly set the 'w' >> writeable flag can be written and modified at runtime. No variables >> diff --git a/env/env.c b/env/env.c >> index 06078c7f374..45e638fcd1f 100644 >> --- a/env/env.c >> +++ b/env/env.c >> @@ -195,6 +195,14 @@ int env_load(void) >> int best_prio = -1; >> int prio; >> >> + if (CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)) { >> + /* >> + * When using a list of writeable variables, the baseline comes >> + * from the built-in default env. So load this first. >> + */ >> + env_set_default(NULL, 0); >> + } >> + >> for (prio = 0; (drv = env_driver_lookup(ENVOP_LOAD, prio)); prio++) { >> int ret; >> >> -- >> 2.35.3 >> > > This looks OK, but please can you write some tests in test/env ? Looking at those, it seems there is nothing at all regarding access flags yet. Any suggestions how to first of all build up that infrastructure are welcome. Then I could add this aspect here on top. Jan -- Siemens AG, Technology Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] env: Complete generic support for writable list 2023-02-07 5:49 ` Jan Kiszka @ 2023-02-07 13:38 ` Simon Glass 0 siblings, 0 replies; 9+ messages in thread From: Simon Glass @ 2023-02-07 13:38 UTC (permalink / raw) To: Jan Kiszka Cc: U-Boot Mailing List, Joe Hershberger, Marek Vasut, Stefan Herbrechtsmeier ()Hi Jan, On Mon, 6 Feb 2023 at 22:49, Jan Kiszka <jan.kiszka@siemens.com> wrote: > > On 07.02.23 05:02, Simon Glass wrote: > > Hi Jan, > > > > On Fri, 3 Feb 2023 at 05:23, Jan Kiszka <jan.kiszka@siemens.com> wrote: > >> > >> From: Jan Kiszka <jan.kiszka@siemens.com> > >> > >> This completes what 890feecaab72 started by selecting ENV_APPEND and > >> loading the default env before any other sources. This ensures that load > >> operations pick up all non-writable vars from the default env and only > >> permitted parts from other locations according to the regular > >> priorities. > >> > >> With this change, boards only need to define the list of writable > >> variables but no longer have to provide a custom env_get_location > >> implementation. > >> > >> CC: Joe Hershberger <joe.hershberger@ni.com> > >> CC: Marek Vasut <marex@denx.de> > >> CC: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com> > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > >> Reviewed-by: Marek Vasut <marex@denx.de> > >> --- > >> env/Kconfig | 1 + > >> env/env.c | 8 ++++++++ > >> 2 files changed, 9 insertions(+) > >> > >> diff --git a/env/Kconfig b/env/Kconfig > >> index c409ea71fe5..6e24eee55f2 100644 > >> --- a/env/Kconfig > >> +++ b/env/Kconfig > >> @@ -733,6 +733,7 @@ config ENV_APPEND > >> > >> config ENV_WRITEABLE_LIST > >> bool "Permit write access only to listed variables" > >> + select ENV_APPEND > >> help > >> If defined, only environment variables which explicitly set the 'w' > >> writeable flag can be written and modified at runtime. No variables > >> diff --git a/env/env.c b/env/env.c > >> index 06078c7f374..45e638fcd1f 100644 > >> --- a/env/env.c > >> +++ b/env/env.c > >> @@ -195,6 +195,14 @@ int env_load(void) > >> int best_prio = -1; > >> int prio; > >> > >> + if (CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)) { > >> + /* > >> + * When using a list of writeable variables, the baseline comes > >> + * from the built-in default env. So load this first. > >> + */ > >> + env_set_default(NULL, 0); > >> + } > >> + > >> for (prio = 0; (drv = env_driver_lookup(ENVOP_LOAD, prio)); prio++) { > >> int ret; > >> > >> -- > >> 2.35.3 > >> > > > > This looks OK, but please can you write some tests in test/env ? > > Looking at those, it seems there is nothing at all regarding access > flags yet. Any suggestions how to first of all build up that > infrastructure are welcome. Then I could add this aspect here on top. Yes, starting something a bit new is always harder. My approach is normally to just start small. Anything is better than nothing, and it provides something for others to build on. There are a few tests for hashtable which could be a way to start this. I suggest starting a new env.c file in there with a few env_get() and env_set() calls. You could also use himport_r() to test out different flags. I'm not sure how easy it would be to call env_load() in a test, but it might work. The trickier thing (perhaps to worry about later) is that you are (correctly) using a CONFIG option to enable the feature. For sandbox tests we typically want all features to be enabled at build time (e.g. default y if SANDBOX), then select them at runtime, so we can test them. See test_eth_enabled() for an example of how this is done. Regards, Simon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] env: Complete generic support for writable list 2023-02-03 12:22 ` [PATCH 1/2] env: Complete generic support for writable list Jan Kiszka 2023-02-07 4:02 ` Simon Glass @ 2023-02-10 18:44 ` Tom Rini 1 sibling, 0 replies; 9+ messages in thread From: Tom Rini @ 2023-02-10 18:44 UTC (permalink / raw) To: Jan Kiszka Cc: U-Boot Mailing List, Joe Hershberger, Marek Vasut, Stefan Herbrechtsmeier [-- Attachment #1: Type: text/plain, Size: 884 bytes --] On Fri, Feb 03, 2023 at 01:22:51PM +0100, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > This completes what 890feecaab72 started by selecting ENV_APPEND and > loading the default env before any other sources. This ensures that load > operations pick up all non-writable vars from the default env and only > permitted parts from other locations according to the regular > priorities. > > With this change, boards only need to define the list of writable > variables but no longer have to provide a custom env_get_location > implementation. > > CC: Joe Hershberger <joe.hershberger@ni.com> > CC: Marek Vasut <marex@denx.de> > CC: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > Reviewed-by: Marek Vasut <marex@denx.de> Applied to u-boot/master, thanks! -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] env: Couple networking-related variable flags to CONFIG_NET 2023-02-03 12:22 [PATCH 0/2] env: Enhancements for establishing variable protection Jan Kiszka 2023-02-03 12:22 ` [PATCH 1/2] env: Complete generic support for writable list Jan Kiszka @ 2023-02-03 12:22 ` Jan Kiszka 2023-02-03 18:23 ` Tom Rini 2023-02-10 18:44 ` Tom Rini 1 sibling, 2 replies; 9+ messages in thread From: Jan Kiszka @ 2023-02-03 12:22 UTC (permalink / raw) To: U-Boot Mailing List; +Cc: Joe Hershberger From: Jan Kiszka <jan.kiszka@siemens.com> Boards may set networking variables programmatically, thus may have CONFIG_NET on but CONFIG_CMD_NET off. The IOT2050 is an example. CC: Joe Hershberger <joe.hershberger@ni.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- env/flags.c | 10 +++++----- include/env_flags.h | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/env/flags.c b/env/flags.c index e3e833c4333..e2866361dfe 100644 --- a/env/flags.c +++ b/env/flags.c @@ -22,7 +22,7 @@ #include <env_internal.h> #endif -#ifdef CONFIG_CMD_NET +#ifdef CONFIG_NET #define ENV_FLAGS_NET_VARTYPE_REPS "im" #else #define ENV_FLAGS_NET_VARTYPE_REPS "" @@ -57,7 +57,7 @@ static const char * const env_flags_vartype_names[] = { "decimal", "hexadecimal", "boolean", -#ifdef CONFIG_CMD_NET +#ifdef CONFIG_NET "IP address", "MAC address", #endif @@ -211,7 +211,7 @@ static void skip_num(int hex, const char *value, const char **end, *end = value; } -#ifdef CONFIG_CMD_NET +#ifdef CONFIG_NET int eth_validate_ethaddr_str(const char *addr) { const char *end; @@ -244,7 +244,7 @@ static int _env_flags_validate_type(const char *value, enum env_flags_vartype type) { const char *end; -#ifdef CONFIG_CMD_NET +#ifdef CONFIG_NET const char *cur; int i; #endif @@ -273,7 +273,7 @@ static int _env_flags_validate_type(const char *value, if (value[1] != '\0') return -1; break; -#ifdef CONFIG_CMD_NET +#ifdef CONFIG_NET case env_flags_vartype_ipaddr: cur = value; for (i = 0; i < 4; i++) { diff --git a/include/env_flags.h b/include/env_flags.h index 6bd574c2bdb..7de58cc57c3 100644 --- a/include/env_flags.h +++ b/include/env_flags.h @@ -12,7 +12,7 @@ enum env_flags_vartype { env_flags_vartype_decimal, env_flags_vartype_hex, env_flags_vartype_bool, -#ifdef CONFIG_CMD_NET +#ifdef CONFIG_NET env_flags_vartype_ipaddr, env_flags_vartype_macaddr, #endif @@ -121,7 +121,7 @@ enum env_flags_varaccess env_flags_parse_varaccess(const char *flags); */ enum env_flags_varaccess env_flags_parse_varaccess_from_binflags(int binflags); -#ifdef CONFIG_CMD_NET +#ifdef CONFIG_NET /* * Check if a string has the format of an Ethernet MAC address */ -- 2.35.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] env: Couple networking-related variable flags to CONFIG_NET 2023-02-03 12:22 ` [PATCH 2/2] env: Couple networking-related variable flags to CONFIG_NET Jan Kiszka @ 2023-02-03 18:23 ` Tom Rini 2023-02-10 18:44 ` Tom Rini 1 sibling, 0 replies; 9+ messages in thread From: Tom Rini @ 2023-02-03 18:23 UTC (permalink / raw) To: Jan Kiszka; +Cc: U-Boot Mailing List, Joe Hershberger [-- Attachment #1: Type: text/plain, Size: 415 bytes --] On Fri, Feb 03, 2023 at 01:22:52PM +0100, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Boards may set networking variables programmatically, thus may have > CONFIG_NET on but CONFIG_CMD_NET off. The IOT2050 is an example. > > CC: Joe Hershberger <joe.hershberger@ni.com> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Reviewed-by: Tom Rini <trini@konsulko.com> -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] env: Couple networking-related variable flags to CONFIG_NET 2023-02-03 12:22 ` [PATCH 2/2] env: Couple networking-related variable flags to CONFIG_NET Jan Kiszka 2023-02-03 18:23 ` Tom Rini @ 2023-02-10 18:44 ` Tom Rini 1 sibling, 0 replies; 9+ messages in thread From: Tom Rini @ 2023-02-10 18:44 UTC (permalink / raw) To: Jan Kiszka; +Cc: U-Boot Mailing List, Joe Hershberger [-- Attachment #1: Type: text/plain, Size: 452 bytes --] On Fri, Feb 03, 2023 at 01:22:52PM +0100, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Boards may set networking variables programmatically, thus may have > CONFIG_NET on but CONFIG_CMD_NET off. The IOT2050 is an example. > > CC: Joe Hershberger <joe.hershberger@ni.com> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > Reviewed-by: Tom Rini <trini@konsulko.com> Applied to u-boot/master, thanks! -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-02-10 18:47 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-03 12:22 [PATCH 0/2] env: Enhancements for establishing variable protection Jan Kiszka 2023-02-03 12:22 ` [PATCH 1/2] env: Complete generic support for writable list Jan Kiszka 2023-02-07 4:02 ` Simon Glass 2023-02-07 5:49 ` Jan Kiszka 2023-02-07 13:38 ` Simon Glass 2023-02-10 18:44 ` Tom Rini 2023-02-03 12:22 ` [PATCH 2/2] env: Couple networking-related variable flags to CONFIG_NET Jan Kiszka 2023-02-03 18:23 ` Tom Rini 2023-02-10 18:44 ` Tom Rini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox