| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAK7LNAROnZpZiOC4eS5kTcv4Q2YDrE9KYBD-dVcfXwBPQWvbmg@mail.gmail.com>
Date: Tue, 29 Aug 2023 22:13:42 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Sergey Senozhatsky <senozhatsky@...omium.org>
Cc: Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Nicolas Schier <nicolas@...sle.eu>,
Jonathan Corbet <corbet@....net>,
Tomasz Figa <tfiga@...omium.org>, linux-kbuild@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kconfig: add warn-unknown-symbols sanity check
On Sun, Aug 27, 2023 at 6:00 PM Sergey Senozhatsky
<senozhatsky@...omium.org> wrote:
>
> Introduce KCONFIG_WARN_UNKNOWN_SYMBOLS environment variable,
> which makes Kconfig warn about unknown .config symbols.
>
> This is especially useful for continuous kernel uprevs when
> some symbols can be either removed or renamed between kernel
> releases (which can go unnoticed otherwise).
>
> By default KCONFIG_WARN_UNKNOWN_SYMBOLS generates warnings,
> which are non-terminal. There is an additional environment
> variable KCONFIG_WERROR that overrides this behaviour and
> turns warnings into errors.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@...omium.org>
> ---
> Documentation/kbuild/kconfig.rst | 11 +++++++++++
> scripts/kconfig/confdata.c | 23 +++++++++++++++++++++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/Documentation/kbuild/kconfig.rst b/Documentation/kbuild/kconfig.rst
> index 6530ecd99da3..4de1f5435b7b 100644
> --- a/Documentation/kbuild/kconfig.rst
> +++ b/Documentation/kbuild/kconfig.rst
> @@ -56,6 +56,17 @@ KCONFIG_OVERWRITECONFIG
> If you set KCONFIG_OVERWRITECONFIG in the environment, Kconfig will not
> break symlinks when .config is a symlink to somewhere else.
>
> +KCONFIG_WARN_UNKNOWN_SYMBOLS
> +----------------------------
> +This environment variable makes Kconfig warn about all unrecognized
> +symbols in the .config file.
This warns not only for the .config but also defconfig files.
Could you reword it?
For example,
"symbols in the config input".
> +
> +KCONFIG_WERROR
> +--------------
> +If set, Kconfig will treat `KCONFIG_WARN_UNKNOWN_SYMBOLS` warnings as
> +errors.
My hope is to turn other warnings in the config file into errors.
See below.
> +
> +
> `CONFIG_`
> ---------
> If you set `CONFIG_` in the environment, Kconfig will prefix all symbols
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 992575f1e976..c24f637827fe 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -349,7 +349,12 @@ int conf_read_simple(const char *name, int def)
> char *p, *p2;
> struct symbol *sym;
> int i, def_flags;
> + bool found_unknown = false;
> + const char *warn_unknown;
> + const char *werror;
>
> + warn_unknown = getenv("KCONFIG_WARN_UNKNOWN_SYMBOLS");
> + werror = getenv("KCONFIG_WERROR");
> if (name) {
> in = zconf_fopen(name);
> } else {
> @@ -437,6 +442,13 @@ int conf_read_simple(const char *name, int def)
> if (def == S_DEF_USER) {
> sym = sym_find(line + 2 + strlen(CONFIG_));
> if (!sym) {
> + if (warn_unknown) {
> + conf_warning("unknown symbol: %s",
> + line + 2 + strlen(CONFIG_));
> + found_unknown = true;
> + continue;
Please drop this 'continue' because it would skip
conf_set_changed(true).
warn_unknown should keep the same code flow
except showing the warning message.
> + }
> +
> conf_set_changed(true);
> continue;
> }
> @@ -471,6 +483,13 @@ int conf_read_simple(const char *name, int def)
>
> sym = sym_find(line + strlen(CONFIG_));
> if (!sym) {
> + if (warn_unknown && def != S_DEF_AUTO) {
> + conf_warning("unknown symbol: %s",
> + line + strlen(CONFIG_));
> + found_unknown = true;
> + continue;
Same here.
When KCONFIG_WARN_UNKNOWN_SYMBOLS is set,
conf_set_changed(true) will be skipped.
You can do like this:
@@ -471,7 +483,7 @@ int conf_read_simple(const char *name, int def)
sym = sym_find(line + strlen(CONFIG_));
if (!sym) {
- if (def == S_DEF_AUTO)
+ if (def == S_DEF_AUTO) {
/*
* Reading from include/config/auto.conf
* If CONFIG_FOO previously existed in
@@ -479,8 +491,13 @@ int conf_read_simple(const char *name, int def)
* include/config/FOO must be touched.
*/
conf_touch_dep(line + strlen(CONFIG_));
- else
+ } else {
+ if (warn_unknown && def != S_DEF_AUTO)
+ conf_warning("unknown
symbol: %s",
+ line +
strlen(CONFIG_));
+
conf_set_changed(true);
+ }
continue;
}
> + }
> +
> if (def == S_DEF_AUTO)
> /*
> * Reading from include/config/auto.conf
> @@ -519,6 +538,10 @@ int conf_read_simple(const char *name, int def)
> }
> free(line);
> fclose(in);
> +
> + if (found_unknown && werror)
> + exit(1);
I like to reuse 'conf_warnings' as you did in the previous version.
if (conf_warnings && werror)
exit(1)
Then, you do not need to add 'found_unknown'.
--
Best Regards
Masahiro Yamada
Powered by blists - more mailing lists