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
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAK7LNAQmcY01i+feMGDEAZrhVrTh8v-4sis75-gaoSYZxF+p6Q@mail.gmail.com>
Date: Thu, 27 Jun 2024 04:17:22 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: HONG Yifan <elsk@...gle.com>
Cc: kernel-team@...roid.com, linux-kbuild@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] kconfig: recursive checks drop file/lineno

On Fri, Jun 21, 2024 at 6:11 AM HONG Yifan <elsk@...gle.com> wrote:
>
> This prevents segfault when getting filename
> and lineno in recursive checks.
>
> If the following snippet is found in Kconfig:
>
> [Test code 1]
>
> config FOO
>        tristate
>        depends on BAR
>        select BAR
>        help
>          foo



config FOO
        bool
        depends on BAR
        select BAR


is enough to produce this issue.




> ... without BAR defined; then if one runs
> `make tinyconfig`, there is a segfault.

There is no need to provide the particular command
because this issue occurs for any 'make *config' command.





>
>   Kconfig:34:error: recursive dependency detected!
>   Kconfig:34:   symbol FOO depends on BAR
>   make[4]: *** [scripts/kconfig/Makefile:85: allnoconfig] Segmentation fault
>
> This is because of the following. BAR is
> a fake entry created by sym_lookup() with prop
> being NULL. In the recursive check, there is a
> NULL check for prop to fall back to
> stack->sym->prop if stack->prop is NULL. However,
> in this case, stack->sym points to the fake BAR
> entry created by sym_lookup(), so prop is still
> NULL. prop was then referenced without additional
> NULL checks, causing segfault.


Each line is too short.

You can wrap lines around 70-columns.



>
> As the previous email thread suggests, the file
> and lineno for select is also wrong:
>
> [Test code 2]
>
> config FOO
>        bool
>
> config BAR
>        bool
>
> config FOO
>        bool "FOO"
>        depends on BAR
>        select BAR
>
> $ make defconfig
> *** Default configuration is based on 'x86_64_defconfig'
> Kconfig:1:error: recursive dependency detected!
> Kconfig:1: symbol FOO depends on BAR
> Kconfig:4: symbol BAR is selected by FOO
> [...]
>
> Kconfig:4 should be Kconfig:10.
>
> This patch deletes the wrong and segfault-prone
> filename/lineno inference completely. With this
> patch, Test code 1 yields:
>
> error: recursive dependency detected!
>         symbol FOO depends on BAR
>         symbol BAR is selected by FOO
>
> Link: https://lore.kernel.org/linux-kbuild/20240618185609.4096399-1-elsk@google.com/
> Signed-off-by: HONG Yifan <elsk@...gle.com>
>
> --
> v2: Delete all filenames/lineno completely as
> suggested by masahiroy@...nel.org


This will break 'make testconfig' because you did not adjust

scripts/kconfig/tests/err_recursive_dep/expected_stderr




I cleaned up the function a little more.

Please adjust scripts/kconfig/tests/err_recursive_dep/expected_stderr
and rebase the patch on top of this:

https://lore.kernel.org/linux-kbuild/20240626182212.3758235-1-masahiroy@kernel.org/T/#t




> ---
>  scripts/kconfig/symbol.c | 43 +++++++++++++++-------------------------
>  1 file changed, 16 insertions(+), 27 deletions(-)
>
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 8df0a75f40b9..3974e46a8533 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -1042,9 +1042,9 @@ static void sym_check_print_recursive(struct symbol *last_sym)
>  {
>         struct dep_stack *stack;
>         struct symbol *sym, *next_sym;
> -       struct menu *menu = NULL;
>         struct property *prop;
>         struct dep_stack cv_stack;
> +       enum prop_type type;
>
>         if (sym_is_choice_value(last_sym)) {
>                 dep_stack_insert(&cv_stack, last_sym);
> @@ -1066,54 +1066,43 @@ static void sym_check_print_recursive(struct symbol *last_sym)
>                 if (prop == NULL)
>                         prop = stack->sym->prop;


Please remove these two lines as well.
It is useless for getting 'type'.






>
> -               /* for choice values find the menu entry (used below) */
> -               if (sym_is_choice(sym) || sym_is_choice_value(sym)) {
> -                       for (prop = sym->prop; prop; prop = prop->next) {
> -                               menu = prop->menu;
> -                               if (prop->menu)
> -                                       break;
> -                       }
> -               }
> +               if (prop == NULL)
> +                       type = P_UNKNOWN;
> +               else
> +                       type = prop->type;
> +
>                 if (stack->sym == last_sym)
> -                       fprintf(stderr, "%s:%d:error: recursive dependency detected!\n",
> -                               prop->filename, prop->lineno);
> +                       fprintf(stderr, "error: recursive dependency detected!\n");
>
>                 if (sym_is_choice(sym)) {
> -                       fprintf(stderr, "%s:%d:\tchoice %s contains symbol %s\n",
> -                               menu->filename, menu->lineno,
> +                       fprintf(stderr, "\tchoice %s contains symbol %s\n",
>                                 sym->name ? sym->name : "<choice>",
>                                 next_sym->name ? next_sym->name : "<choice>");
>                 } else if (sym_is_choice_value(sym)) {
> -                       fprintf(stderr, "%s:%d:\tsymbol %s is part of choice %s\n",
> -                               menu->filename, menu->lineno,
> +                       fprintf(stderr, "\tsymbol %s is part of choice %s\n",
>                                 sym->name ? sym->name : "<choice>",
>                                 next_sym->name ? next_sym->name : "<choice>");
>                 } else if (stack->expr == &sym->dir_dep.expr) {
> -                       fprintf(stderr, "%s:%d:\tsymbol %s depends on %s\n",
> -                               prop->filename, prop->lineno,
> +                       fprintf(stderr, "\tsymbol %s depends on %s\n",
>                                 sym->name ? sym->name : "<choice>",
>                                 next_sym->name ? next_sym->name : "<choice>");
>                 } else if (stack->expr == &sym->rev_dep.expr) {
> -                       fprintf(stderr, "%s:%d:\tsymbol %s is selected by %s\n",
> -                               prop->filename, prop->lineno,
> +                       fprintf(stderr, "\tsymbol %s is selected by %s\n",
>                                 sym->name ? sym->name : "<choice>",
>                                 next_sym->name ? next_sym->name : "<choice>");
>                 } else if (stack->expr == &sym->implied.expr) {
> -                       fprintf(stderr, "%s:%d:\tsymbol %s is implied by %s\n",
> -                               prop->filename, prop->lineno,
> +                       fprintf(stderr, "\tsymbol %s is implied by %s\n",
>                                 sym->name ? sym->name : "<choice>",
>                                 next_sym->name ? next_sym->name : "<choice>");
>                 } else if (stack->expr) {
> -                       fprintf(stderr, "%s:%d:\tsymbol %s %s value contains %s\n",
> -                               prop->filename, prop->lineno,
> +                       fprintf(stderr, "\tsymbol %s %s value contains %s\n",
>                                 sym->name ? sym->name : "<choice>",
> -                               prop_get_type_name(prop->type),
> +                               prop_get_type_name(type),
>                                 next_sym->name ? next_sym->name : "<choice>");
>                 } else {
> -                       fprintf(stderr, "%s:%d:\tsymbol %s %s is visible depending on %s\n",
> -                               prop->filename, prop->lineno,
> +                       fprintf(stderr, "\tsymbol %s %s is visible depending on %s\n",
>                                 sym->name ? sym->name : "<choice>",
> -                               prop_get_type_name(prop->type),
> +                               prop_get_type_name(type),
>                                 next_sym->name ? next_sym->name : "<choice>");
>                 }
>         }
> --
> 2.45.2.741.gdbec12cfda-goog
>


--
Best Regards
Masahiro Yamada

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ