| Message ID | 20210215181511.2840674-3-mic@digikod.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Kconfig oldconfig string update | expand |
On Tue, Feb 16, 2021 at 3:14 AM Mickaël Salaün <mic@digikod.net> wrote: > > From: Mickaël Salaün <mic@linux.microsoft.com> > > Content of string configuration may depend on related kernel > configurations. Modify oldconfig and syncconfig to inform users about > possible required configuration update and give them the opportunity to > update it: > * if dependencies of this string has changed (e.g. enabled or disabled), > * and if the current value of this string is different than the (new) > default one. > > This is particularly relevant for CONFIG_LSM which contains a list of > LSMs enabled at boot, but users will not have a chance to update this > list with a make oldconfig. If CONFIG_LSM already exists in the .config, oldconfig does not show a prompt. This is the expected behavior. You are trying to fix your problem in a wrong way. NACK. > > Cc: Casey Schaufler <casey@schaufler-ca.com> > Cc: James Morris <jmorris@namei.org> > Cc: Masahiro Yamada <masahiroy@kernel.org> > Cc: Serge E. Hallyn <serge@hallyn.com> > Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com> > Link: https://lore.kernel.org/r/20210215181511.2840674-3-mic@digikod.net > --- > scripts/kconfig/conf.c | 37 ++++++++++++++++++++++++++++++++++--- > 1 file changed, 34 insertions(+), 3 deletions(-) > > diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c > index 18a233d27a8d..8633dacd39a9 100644 > --- a/scripts/kconfig/conf.c > +++ b/scripts/kconfig/conf.c > @@ -82,6 +82,26 @@ static void xfgets(char *str, int size, FILE *in) > printf("%s", str); > } > > +static bool may_need_string_update(struct symbol *sym, const char *def) > +{ > + const struct symbol *dep_sym; > + const struct expr *e; > + > + if (sym->type != S_STRING) > + return false; > + if (strcmp(def, sym_get_string_default(sym)) == 0) > + return false; > + /* > + * The user may want to synchronize the content of a string related to > + * changed dependencies (e.g. CONFIG_LSM). > + */ > + expr_list_for_each_sym(sym->dir_dep.expr, e, dep_sym) { > + if (dep_sym->flags & SYMBOL_CHANGED) > + return true; > + } > + return false; > +} > + > static int conf_askvalue(struct symbol *sym, const char *def) > { > enum symbol_type type = sym_get_type(sym); > @@ -102,7 +122,7 @@ static int conf_askvalue(struct symbol *sym, const char *def) > switch (input_mode) { > case oldconfig: > case syncconfig: > - if (sym_has_value(sym)) { > + if (sym_has_value(sym) && !may_need_string_update(sym, def)) { > printf("%s\n", def); > return 0; > } > @@ -137,8 +157,19 @@ static int conf_string(struct menu *menu) > printf("%*s%s ", indent - 1, "", menu->prompt->text); > printf("(%s) ", sym->name); > def = sym_get_string_value(sym); > - if (def) > - printf("[%s] ", def); > + if (def) { > + if (may_need_string_update(sym, def)) { > + indent += 2; > + printf("\n%*sDefault value is [%s]\n", > + indent - 1, "", > + sym_get_string_default(sym)); > + printf("%*sCurrent value is [%s] ", > + indent - 1, "", def); > + indent -= 2; > + } else { > + printf("[%s] ", def); > + } > + } > if (!conf_askvalue(sym, def)) > return 0; > switch (line[0]) { > -- > 2.30.0 >
On 21/02/2021 09:47, Masahiro Yamada wrote: > On Tue, Feb 16, 2021 at 3:14 AM Mickaël Salaün <mic@digikod.net> wrote: >> >> From: Mickaël Salaün <mic@linux.microsoft.com> >> >> Content of string configuration may depend on related kernel >> configurations. Modify oldconfig and syncconfig to inform users about >> possible required configuration update and give them the opportunity to >> update it: >> * if dependencies of this string has changed (e.g. enabled or disabled), >> * and if the current value of this string is different than the (new) >> default one. >> >> This is particularly relevant for CONFIG_LSM which contains a list of >> LSMs enabled at boot, but users will not have a chance to update this >> list with a make oldconfig. > > If CONFIG_LSM already exists in the .config, > oldconfig does not show a prompt. > This is the expected behavior. It is not the behavior wished for LSM stacking. > > You are trying to fix your problem in a wrong way. > NACK. What do you suggest to ensure that users will be asked to update the CONFIG_LSM string if they add or remove an LSM? > > > >> >> Cc: Casey Schaufler <casey@schaufler-ca.com> >> Cc: James Morris <jmorris@namei.org> >> Cc: Masahiro Yamada <masahiroy@kernel.org> >> Cc: Serge E. Hallyn <serge@hallyn.com> >> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com> >> Link: https://lore.kernel.org/r/20210215181511.2840674-3-mic@digikod.net >> --- >> scripts/kconfig/conf.c | 37 ++++++++++++++++++++++++++++++++++--- >> 1 file changed, 34 insertions(+), 3 deletions(-) >> >> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c >> index 18a233d27a8d..8633dacd39a9 100644 >> --- a/scripts/kconfig/conf.c >> +++ b/scripts/kconfig/conf.c >> @@ -82,6 +82,26 @@ static void xfgets(char *str, int size, FILE *in) >> printf("%s", str); >> } >> >> +static bool may_need_string_update(struct symbol *sym, const char *def) >> +{ >> + const struct symbol *dep_sym; >> + const struct expr *e; >> + >> + if (sym->type != S_STRING) >> + return false; >> + if (strcmp(def, sym_get_string_default(sym)) == 0) >> + return false; >> + /* >> + * The user may want to synchronize the content of a string related to >> + * changed dependencies (e.g. CONFIG_LSM). >> + */ >> + expr_list_for_each_sym(sym->dir_dep.expr, e, dep_sym) { >> + if (dep_sym->flags & SYMBOL_CHANGED) >> + return true; >> + } >> + return false; >> +} >> + >> static int conf_askvalue(struct symbol *sym, const char *def) >> { >> enum symbol_type type = sym_get_type(sym); >> @@ -102,7 +122,7 @@ static int conf_askvalue(struct symbol *sym, const char *def) >> switch (input_mode) { >> case oldconfig: >> case syncconfig: >> - if (sym_has_value(sym)) { >> + if (sym_has_value(sym) && !may_need_string_update(sym, def)) { >> printf("%s\n", def); >> return 0; >> } >> @@ -137,8 +157,19 @@ static int conf_string(struct menu *menu) >> printf("%*s%s ", indent - 1, "", menu->prompt->text); >> printf("(%s) ", sym->name); >> def = sym_get_string_value(sym); >> - if (def) >> - printf("[%s] ", def); >> + if (def) { >> + if (may_need_string_update(sym, def)) { >> + indent += 2; >> + printf("\n%*sDefault value is [%s]\n", >> + indent - 1, "", >> + sym_get_string_default(sym)); >> + printf("%*sCurrent value is [%s] ", >> + indent - 1, "", def); >> + indent -= 2; >> + } else { >> + printf("[%s] ", def); >> + } >> + } >> if (!conf_askvalue(sym, def)) >> return 0; >> switch (line[0]) { >> -- >> 2.30.0 >> > >
On Sun, Feb 21, 2021 at 8:09 PM Mickaël Salaün <mic@digikod.net> wrote: > > > On 21/02/2021 09:47, Masahiro Yamada wrote: > > On Tue, Feb 16, 2021 at 3:14 AM Mickaël Salaün <mic@digikod.net> wrote: > >> > >> From: Mickaël Salaün <mic@linux.microsoft.com> > >> > >> Content of string configuration may depend on related kernel > >> configurations. Modify oldconfig and syncconfig to inform users about > >> possible required configuration update and give them the opportunity to > >> update it: > >> * if dependencies of this string has changed (e.g. enabled or disabled), > >> * and if the current value of this string is different than the (new) > >> default one. > >> > >> This is particularly relevant for CONFIG_LSM which contains a list of > >> LSMs enabled at boot, but users will not have a chance to update this > >> list with a make oldconfig. > > > > If CONFIG_LSM already exists in the .config, > > oldconfig does not show a prompt. > > This is the expected behavior. > > It is not the behavior wished for LSM stacking. Because LSM is doing wrong. > > > > You are trying to fix your problem in a wrong way. > > NACK. > > What do you suggest to ensure that users will be asked to update the > CONFIG_LSM string if they add or remove an LSM? Fix it in the security subsystem. Hint: See 050e9baa9dc9fbd9ce2b27f0056990fc9e0a08a0
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 18a233d27a8d..8633dacd39a9 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -82,6 +82,26 @@ static void xfgets(char *str, int size, FILE *in) printf("%s", str); } +static bool may_need_string_update(struct symbol *sym, const char *def) +{ + const struct symbol *dep_sym; + const struct expr *e; + + if (sym->type != S_STRING) + return false; + if (strcmp(def, sym_get_string_default(sym)) == 0) + return false; + /* + * The user may want to synchronize the content of a string related to + * changed dependencies (e.g. CONFIG_LSM). + */ + expr_list_for_each_sym(sym->dir_dep.expr, e, dep_sym) { + if (dep_sym->flags & SYMBOL_CHANGED) + return true; + } + return false; +} + static int conf_askvalue(struct symbol *sym, const char *def) { enum symbol_type type = sym_get_type(sym); @@ -102,7 +122,7 @@ static int conf_askvalue(struct symbol *sym, const char *def) switch (input_mode) { case oldconfig: case syncconfig: - if (sym_has_value(sym)) { + if (sym_has_value(sym) && !may_need_string_update(sym, def)) { printf("%s\n", def); return 0; } @@ -137,8 +157,19 @@ static int conf_string(struct menu *menu) printf("%*s%s ", indent - 1, "", menu->prompt->text); printf("(%s) ", sym->name); def = sym_get_string_value(sym); - if (def) - printf("[%s] ", def); + if (def) { + if (may_need_string_update(sym, def)) { + indent += 2; + printf("\n%*sDefault value is [%s]\n", + indent - 1, "", + sym_get_string_default(sym)); + printf("%*sCurrent value is [%s] ", + indent - 1, "", def); + indent -= 2; + } else { + printf("[%s] ", def); + } + } if (!conf_askvalue(sym, def)) return 0; switch (line[0]) {