[PATCH v1 0/3] Kconfig oldconfig string update

[PATCH v1 0/3] Kconfig oldconfig string update

[Mickaël Salaün]

Hi,

This patch series gives the opportunity to users, when running make
oldconfig, to update configuration strings (e.g. CONFIG_LSM) according
to dependency changes.  This helps users keep a consistent up-to-date
kernel configuration.

This patch series can be applied on v5.11 .

Regards,

Mickaël Salaün (3):
  kconfig: Remove duplicate call to sym_get_string_value()
  kconfig: Ask user if string needs to be changed when dependency
    changed
  security: Add LSMs dependencies to CONFIG_LSM

 scripts/kconfig/conf.c | 37 ++++++++++++++++++++++++++++++++++---
 security/Kconfig       |  4 ++++
 2 files changed, 38 insertions(+), 3 deletions(-)


base-commit: f40ddce88593482919761f74910f42f4b84c004b
-- 
2.30.0

[PATCH v1 1/3] kconfig: Remove duplicate call to sym_get_string_value()

[Mickaël Salaün]

From: Mickaël Salaün <mic@linux.microsoft.com>

Use the saved returned value of sym_get_string_value() instead of
calling it twice.

Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20210215122513.1773897-2-mic@digikod.net
---
 scripts/kconfig/conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index db03e2f45de4..18a233d27a8d 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -137,7 +137,7 @@ 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 (sym_get_string_value(sym))
+		if (def)
 			printf("[%s] ", def);
 		if (!conf_askvalue(sym, def))
 			return 0;
-- 
2.30.0

[PATCH v1 2/3] kconfig: Ask user if string needs to be changed when dependency changed

[Mickaël Salaün]

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.

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/20210215122513.1773897-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

Re: [PATCH v1 2/3] kconfig: Ask user if string needs to be changed when dependency changed

[Boris Kolpackov]

Mickaël Salaün <mic@digikod.net> writes:

> 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.

I have a number of questions:

1. Why is a change in dependencies necessarily means that the dependent's
   value must be revised? Here is a specific example (to make sure we are
   talking about the same things):

   config FOO
     string "Foo value"
     depends on BAR || BAZ

   Why, in the general case, when I disable BAR and enable BAZ I must
   also revise the value of FOO?

2. How do you know that what's in the user's .config is the old default
   and in Kconfig -- the new default value? What if in the user's .config
   is a custom value (with which the user is perfectly happy) and what's
   in Kconfig is the old default (which the user has already seen)?

3. Why limit this to strings only?


> 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 my understanding above is correct, this feels like it's been purpose-
made to address whatever issue you are having with CONFIG_LSM. If so,
what about potential numerous other options that don't have this issue
but will now be presented to the user for modification?

Re: [PATCH v1 2/3] kconfig: Ask user if string needs to be changed when dependency changed

[Mickaël Salaün]

On 15/02/2021 15:13, Boris Kolpackov wrote:
> Mickaël Salaün <mic@digikod.net> writes:
> 
>> 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.
> 
> I have a number of questions:
> 
> 1. Why is a change in dependencies necessarily means that the dependent's
>    value must be revised? Here is a specific example (to make sure we are
>    talking about the same things):
> 
>    config FOO
>      string "Foo value"
>      depends on BAR || BAZ
> 
>    Why, in the general case, when I disable BAR and enable BAZ I must
>    also revise the value of FOO?

It may be necessary, or not, depending of the use of the string. This
semantic is not clearly expressed by kconfig but looking at the current
configuration, there is only 4 strings depending on more than one
dependency:
* SIMDISK1_FILENAME for arch/xtensa
* CMDLINE for arch/sh
* SECURITY_TOMOYO_POLICY_LOADER
* SECURITY_TOMOYO_ACTIVATION_TRIGGER

Such patterns seem in line with this patch.

> 
> 2. How do you know that what's in the user's .config is the old default
>    and in Kconfig -- the new default value? What if in the user's .config
>    is a custom value (with which the user is perfectly happy) and what's
>    in Kconfig is the old default (which the user has already seen)?

The current behavior (i.e. keeping the current user config) is not
changed. The oldconfig target only stops when a string may require an
update, shows to the user the (potentially new but not necessary best)
default value along with the value already in place in the .config file,
and if the user just type enter this current value will not be changed.

> 
> 3. Why limit this to strings only?

Strings contain configuration blobs that may be interpreted by the
kernel but not by kconfig (cf. CONFIG_LSM). It will still be possible to
handle other types if there is some related use cases.

> 
> 
>> 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 my understanding above is correct, this feels like it's been purpose-
> made to address whatever issue you are having with CONFIG_LSM. If so,
> what about potential numerous other options that don't have this issue
> but will now be presented to the user for modification?

This patch series helps address the LSM stacking issue. The 4 other
cases may benefit from this patch too.

[PATCH v1 3/3] security: Add LSMs dependencies to CONFIG_LSM

[Mickaël Salaün]

From: Mickaël Salaün <mic@linux.microsoft.com>

Thanks to the previous commit, this gives the opportunity to users, when
running make oldconfig, to update the list of enabled LSMs at boot time
if an LSM has just been enabled or disabled in the build.  Moreover,
this list only makes sense if at least one LSM is enabled.

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/20210215122513.1773897-4-mic@digikod.net
---
 security/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/security/Kconfig b/security/Kconfig
index 7561f6f99f1d..2bc9ff351176 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -277,6 +277,10 @@ endchoice
 
 config LSM
 	string "Ordered list of enabled LSMs"
+	depends on SECURITY_LOCKDOWN_LSM || SECURITY_YAMA || SECURITY_LOADPIN || \
+		SECURITY_SAFESETID || INTEGRITY || SECURITY_SELINUX || \
+		SECURITY_SMACK || SECURITY_TOMOYO || SECURITY_APPARMOR || \
+		BPF_LSM
 	default "lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
 	default "lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
 	default "lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
-- 
2.30.0

Re: [PATCH v1 3/3] security: Add LSMs dependencies to CONFIG_LSM

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1973 bytes --]

Hi "Mickaël,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on f40ddce88593482919761f74910f42f4b84c004b]

url:    https://github.com/0day-ci/linux/commits/Micka-l-Sala-n/Kconfig-oldconfig-string-update/20210215-203522
base:   f40ddce88593482919761f74910f42f4b84c004b
config: x86_64-randconfig-a005-20210215 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c9439ca36342fb6013187d0a69aef92736951476)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/57f88038e4ac44e3de063cd5914d91cbb3eecf8f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Micka-l-Sala-n/Kconfig-oldconfig-string-update/20210215-203522
        git checkout 57f88038e4ac44e3de063cd5914d91cbb3eecf8f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> security/security.c:85:59: error: use of undeclared identifier 'CONFIG_LSM'
   static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
                                                             ^
   1 error generated.


vim +/CONFIG_LSM +85 security/security.c

^1da177e4c3f41 Linus Torvalds 2005-04-16  84  
13e735c0e95324 Kees Cook      2018-10-09 @85  static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
13e735c0e95324 Kees Cook      2018-10-09  86  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31869 bytes --]

Re: [PATCH v1 3/3] security: Add LSMs dependencies to CONFIG_LSM

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2018 bytes --]

Hi "Mickaël,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on f40ddce88593482919761f74910f42f4b84c004b]

url:    https://github.com/0day-ci/linux/commits/Micka-l-Sala-n/Kconfig-oldconfig-string-update/20210215-203522
base:   f40ddce88593482919761f74910f42f4b84c004b
config: x86_64-randconfig-a005-20210215 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c9439ca36342fb6013187d0a69aef92736951476)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/57f88038e4ac44e3de063cd5914d91cbb3eecf8f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Micka-l-Sala-n/Kconfig-oldconfig-string-update/20210215-203522
        git checkout 57f88038e4ac44e3de063cd5914d91cbb3eecf8f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> security/security.c:85:59: error: use of undeclared identifier 'CONFIG_LSM'
   static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
                                                             ^
   1 error generated.


vim +/CONFIG_LSM +85 security/security.c

^1da177e4c3f41 Linus Torvalds 2005-04-16  84  
13e735c0e95324 Kees Cook      2018-10-09 @85  static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
13e735c0e95324 Kees Cook      2018-10-09  86  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 31869 bytes --]