Hi Marek,
On Wed, 2020-06-03 at 02:01 +0200, Marek Vasut wrote:
> This option marks any U-Boot variable which does not have explicit 'w'
> writeable flag set as read-only. This way the environment can be locked
> down and only variables explicitly configured to be writeable can ever
> be changed by either 'env import', 'env set' or loading user environment
> from environment storage.
I haven't yet been able to get to the bottom of it but this patch seems to
regress the `envtools` build for me. Here is the rather weird error message:
HOSTCC tools/env/fw_env_main.o
In file included from tools/env/fw_env.c:15:
include/env_flags.h:27:22: error: missing binary operator before token "("
#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
^
Any idea what could be the cause for this?
--
Harald
> Signed-off-by: Marek Vasut <ma...@denx.de>
> ---
> env/Kconfig | 8 ++++
> env/flags.c | 92 +++++++++++++++++++++++++++++++++++++++------
> include/env_flags.h | 6 ++-
> lib/hashtable.c | 5 ++-
> 4 files changed, 98 insertions(+), 13 deletions(-)
>
> diff --git a/env/Kconfig b/env/Kconfig
> index 8166e5df91..f53a1457fb 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -613,6 +613,14 @@ config ENV_APPEND
> with newly imported data. This may be used in combination with static
> flags to e.g. to protect variables which must not be modified.
>
> +config ENV_WRITEABLE_LIST
> + bool "Permit write access only to listed variables"
> + default n
> + help
> + If defined, only environment variables which explicitly set the 'w'
> + writeable flag can be written and modified at runtime. No variables
> + can be otherwise created, written or imported into the environment.
> +
> config ENV_ACCESS_IGNORE_FORCE
> bool "Block forced environment operations"
> default n
> diff --git a/env/flags.c b/env/flags.c
> index f7a53775c4..a2f6c1a3ec 100644
> --- a/env/flags.c
> +++ b/env/flags.c
> @@ -28,8 +28,15 @@
> #define ENV_FLAGS_NET_VARTYPE_REPS ""
> #endif
>
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> +#define ENV_FLAGS_WRITEABLE_VARACCESS_REPS "w"
> +#else
> +#define ENV_FLAGS_WRITEABLE_VARACCESS_REPS ""
> +#endif
> +
> static const char env_flags_vartype_rep[] = "sdxb"
> ENV_FLAGS_NET_VARTYPE_REPS;
> -static const char env_flags_varaccess_rep[] = "aroc";
> +static const char env_flags_varaccess_rep[] =
> + "aroc" ENV_FLAGS_WRITEABLE_VARACCESS_REPS;
> static const int env_flags_varaccess_mask[] = {
> 0,
> ENV_FLAGS_VARACCESS_PREVENT_DELETE |
> @@ -38,7 +45,11 @@ static const int env_flags_varaccess_mask[] = {
> ENV_FLAGS_VARACCESS_PREVENT_DELETE |
> ENV_FLAGS_VARACCESS_PREVENT_OVERWR,
> ENV_FLAGS_VARACCESS_PREVENT_DELETE |
> - ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR};
> + ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR,
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + ENV_FLAGS_VARACCESS_WRITEABLE,
> +#endif
> + };
>
> #ifdef CONFIG_CMD_ENV_FLAGS
> static const char * const env_flags_vartype_names[] = {
> @@ -56,6 +67,9 @@ static const char * const env_flags_varaccess_names[] = {
> "read-only",
> "write-once",
> "change-default",
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + "writeable",
> +#endif
> };
>
> /*
> @@ -130,21 +144,33 @@ enum env_flags_vartype env_flags_parse_vartype(const
> char *flags)
> */
> enum env_flags_varaccess env_flags_parse_varaccess(const char *flags)
> {
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + enum env_flags_varaccess va_default = env_flags_varaccess_readonly;
> +#else
> + enum env_flags_varaccess va_default = env_flags_varaccess_any;
> +#endif
> + enum env_flags_varaccess va;
> char *access;
>
> if (strlen(flags) <= ENV_FLAGS_VARACCESS_LOC)
> - return env_flags_varaccess_any;
> + return va_default;
>
> access = strchr(env_flags_varaccess_rep,
> flags[ENV_FLAGS_VARACCESS_LOC]);
>
> - if (access != NULL)
> - return (enum env_flags_varaccess)
> + if (access != NULL) {
> + va = (enum env_flags_varaccess)
> (access - &env_flags_varaccess_rep[0]);
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + if (va != env_flags_varaccess_writeable)
> + return env_flags_varaccess_readonly;
> +#endif
> + return va;
> + }
>
> printf("## Warning: Unknown environment variable access method '%c'\n",
> flags[ENV_FLAGS_VARACCESS_LOC]);
> - return env_flags_varaccess_any;
> + return va_default;
> }
>
> /*
> @@ -152,17 +178,29 @@ enum env_flags_varaccess
> env_flags_parse_varaccess(const char *flags)
> */
> enum env_flags_varaccess env_flags_parse_varaccess_from_binflags(int
> binflags)
> {
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + enum env_flags_varaccess va_default = env_flags_varaccess_readonly;
> +#else
> + enum env_flags_varaccess va_default = env_flags_varaccess_any;
> +#endif
> + enum env_flags_varaccess va;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(env_flags_varaccess_mask); i++)
> if (env_flags_varaccess_mask[i] ==
> - (binflags & ENV_FLAGS_VARACCESS_BIN_MASK))
> - return (enum env_flags_varaccess)i;
> + (binflags & ENV_FLAGS_VARACCESS_BIN_MASK)) {
> + va = (enum env_flags_varaccess)i;
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + if (va != env_flags_varaccess_writeable)
> + return env_flags_varaccess_readonly;
> +#endif
> + return va;
> + }
>
> printf("Warning: Non-standard access flags. (0x%x)\n",
> binflags & ENV_FLAGS_VARACCESS_BIN_MASK);
>
> - return env_flags_varaccess_any;
> + return va_default;
> }
>
> static inline int is_hex_prefix(const char *value)
> @@ -325,14 +363,20 @@ enum env_flags_vartype env_flags_get_type(const char
> *name)
> */
> enum env_flags_varaccess env_flags_get_varaccess(const char *name)
> {
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + const char *flags_list = NULL;
> + enum env_flags_varaccess va_default = env_flags_varaccess_readonly;
> +#else
> const char *flags_list = env_get(ENV_FLAGS_VAR);
> + enum env_flags_varaccess va_default = env_flags_varaccess_any;
> +#endif
> char flags[ENV_FLAGS_ATTR_MAX_LEN + 1];
>
> if (env_flags_lookup(flags_list, name, flags))
> - return env_flags_varaccess_any;
> + return va_default;
>
> if (strlen(flags) <= ENV_FLAGS_VARACCESS_LOC)
> - return env_flags_varaccess_any;
> + return va_default;
>
> return env_flags_parse_varaccess(flags);
> }
> @@ -426,7 +470,11 @@ void env_flags_init(struct env_entry *var_entry)
> int ret = 1;
>
> if (first_call) {
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + flags_list = ENV_FLAGS_LIST_STATIC;
> +#else
> flags_list = env_get(ENV_FLAGS_VAR);
> +#endif
> first_call = 0;
> }
> /* look in the ".flags" and static for a reference to this variable */
> @@ -435,6 +483,16 @@ void env_flags_init(struct env_entry *var_entry)
> /* if any flags were found, set the binary form to the entry */
> if (!ret && strlen(flags))
> var_entry->flags = env_parse_flags_to_bin(flags);
> +
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + /* Anything which is not explicitly writeable is read-only */
> + if (!(var_entry->flags & ENV_FLAGS_VARACCESS_WRITEABLE)) {
> + var_entry->flags &= ~ENV_FLAGS_VARACCESS_BIN_MASK;
> + var_entry->flags |= ENV_FLAGS_VARACCESS_PREVENT_DELETE |
> + ENV_FLAGS_VARACCESS_PREVENT_CREATE |
> + ENV_FLAGS_VARACCESS_PREVENT_OVERWR;
> + }
> +#endif
> }
>
> /*
> @@ -523,6 +581,18 @@ int env_flags_validate(const struct env_entry *item,
> const char *newval,
> }
>
> /* check for access permission */
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + if (flag & H_DEFAULT)
> + return 0; /* Default env is always OK */
> +
> + /* Writeable variables can be overwritten, anything else can not */
> + if (op == env_op_overwrite &&
> + item->flags & ENV_FLAGS_VARACCESS_WRITEABLE)
> + return 0;
> +
> + return 1;
> +#endif
> +
> #ifndef CONFIG_ENV_ACCESS_IGNORE_FORCE
> if (flag & H_FORCE) {
> printf("## Error: Can't force access to \"%s\"\n", name);
> diff --git a/include/env_flags.h b/include/env_flags.h
> index 725841a891..62c3dd9fa0 100644
> --- a/include/env_flags.h
> +++ b/include/env_flags.h
> @@ -24,6 +24,9 @@ enum env_flags_varaccess {
> env_flags_varaccess_readonly,
> env_flags_varaccess_writeonce,
> env_flags_varaccess_changedefault,
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + env_flags_varaccess_writeable,
> +#endif
> env_flags_varaccess_end
> };
>
> @@ -173,6 +176,7 @@ int env_flags_validate(const struct env_entry *item,
> const char *newval,
> #define ENV_FLAGS_VARACCESS_PREVENT_CREATE 0x00000010
> #define ENV_FLAGS_VARACCESS_PREVENT_OVERWR 0x00000020
> #define ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR 0x00000040
> -#define ENV_FLAGS_VARACCESS_BIN_MASK 0x00000078
> +#define ENV_FLAGS_VARACCESS_WRITEABLE 0x00000080
> +#define ENV_FLAGS_VARACCESS_BIN_MASK 0x000000f8
>
> #endif /* __ENV_FLAGS_H__ */
> diff --git a/lib/hashtable.c b/lib/hashtable.c
> index 24aef5a085..bebad9d382 100644
> --- a/lib/hashtable.c
> +++ b/lib/hashtable.c
> @@ -946,9 +946,12 @@ int himport_r(struct hsearch_data *htab,
> e.data = value;
>
> hsearch_r(e, ENV_ENTER, &rv, htab, flag);
> - if (rv == NULL)
> +#if !CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + if (rv == NULL) {
> printf("himport_r: can't insert \"%s=%s\" into hash
> table\n",
> name, value);
> + }
> +#endif
>
> debug("INSERT: table %p, filled %d/%d rv %p ==> name=\"%s\"
> value=\"%s\"\n",
> htab, htab->filled, htab->size,
