Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221023191055.85098-1-masahiroy@kernel.org> <8c04e9bbc8d48946c60ff56ef7f20bef74e3c401.camel@pengutronix.de>
In-Reply-To: <8c04e9bbc8d48946c60ff56ef7f20bef74e3c401.camel@pengutronix.de>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Wed, 2 Nov 2022 17:35:37 +0900
Message-ID: <CAK7LNATzANbYu9E1w5k0BxG_R8TpuZvxUv==T32iiRcfdNrxVA@mail.gmail.com>
Subject: Re: [PATCH] kconfig: fix segmentation fault in menuconfig search
To:     Johannes Zink <j.zink@pengutronix.de>
Cc:     linux-kbuild@vger.kernel.org,
        Ariel Marcovitch <arielmarcovitch@gmail.com>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Nov 1, 2022 at 2:31 AM Johannes Zink <j.zink@pengutronix.de> wrote:
>
> Hi Masahiro,
>
> On Mon, 2022-10-24 at 04:10 +0900, Masahiro Yamada wrote:
> > Since commit d05377e184fc ("kconfig: Create links to main menu items
> > in search"), menuconfig shows a jump key next to "Main menu" if the
> > nearest visible parent is the rootmenu. If you press that jump key,
> > menuconfig crashes with a segmentation fault.
> >
> > For example, do this:
> >
> >   $ make ARCH=arm64 allnoconfig menuconfig
> >
> > Press '/' to search for the string "ACPI". Press '1' to choose
> > "(1) Main menu". Then, menuconfig crashed with a segmentation fault.
> >
> > The following code in search_conf()
> >
> >     conf(targets[i]->parent, targets[i]);
> >
> > results in NULL pointer dereference because targets[i] is the
> > rootmenu,
> > which does not have a parent.
> >
> > Commit d05377e184fc tried to fix the issue of top-level items not
> > having
> > a jump key, but adding the "Main menu" was not the right fix.
> >
> > The correct fix is to show the searched item itself. This fixes
> > another
> > weird behavior described in the comment block.
> >
> > Fixes: d05377e184fc ("kconfig: Create links to main menu items in
> > search")
> > Reported-by: Johannes Zink <j.zink@pengutronix.de>
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>
> Sorry for the delay, I eventually found time to test your patch, and it
> fixes the issue I reported. Thank you for helping out, feel free to add
> my
>
> Tested-by: Johannes Zink <j.zink@pengutronix.de>


Applied to linux-kbuild/fixes.


> > ---
> >
> >  scripts/kconfig/menu.c | 23 ++++-------------------
> >  1 file changed, 4 insertions(+), 19 deletions(-)
> >
> > diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> > index 62b6313f51c8..109325f31bef 100644
> > --- a/scripts/kconfig/menu.c
> > +++ b/scripts/kconfig/menu.c
> > @@ -722,8 +722,8 @@ static void get_prompt_str(struct gstr *r, struct
> > property *prop,
> >         if (!expr_eq(prop->menu->dep, prop->visible.expr))
> >                 get_dep_str(r, prop->visible.expr, "  Visible if: ");
> >
> > -       menu = prop->menu->parent;
> > -       for (i = 0; menu && i < 8; menu = menu->parent) {
> > +       menu = prop->menu;
> > +       for (i = 0; menu != &rootmenu && i < 8; menu = menu->parent)
> > {
> >                 bool accessible = menu_is_visible(menu);
> >
> >                 submenu[i++] = menu;
> > @@ -733,16 +733,7 @@ static void get_prompt_str(struct gstr *r,
> > struct property *prop,
> >         if (head && location) {
> >                 jump = xmalloc(sizeof(struct jump_key));
> >
> > -               if (menu_is_visible(prop->menu)) {
> > -                       /*
> > -                        * There is not enough room to put the hint
> > at the
> > -                        * beginning of the "Prompt" line. Put the
> > hint on the
> > -                        * last "Location" line even when it would
> > belong on
> > -                        * the former.
> > -                        */
> > -                       jump->target = prop->menu;
> > -               } else
> > -                       jump->target = location;
> > +               jump->target = location;
> >
> >                 if (list_empty(head))
> >                         jump->index = 0;
> > @@ -758,13 +749,7 @@ static void get_prompt_str(struct gstr *r,
> > struct property *prop,
> >                 menu = submenu[i];
> >                 if (jump && menu == location)
> >                         jump->offset = strlen(r->s);
> > -
> > -               if (menu == &rootmenu)
> > -                       /* The real rootmenu prompt is ugly */
> > -                       str_printf(r, "%*cMain menu", j, ' ');
> > -               else
> > -                       str_printf(r, "%*c-> %s", j, ' ',
> > menu_get_prompt(menu));
> > -
> > +               str_printf(r, "%*c-> %s", j, ' ',
> > menu_get_prompt(menu));
> >                 if (menu->sym) {
> >                         str_printf(r, " (%s [=%s])", menu->sym->name
> > ?
> >                                 menu->sym->name : "<choice>",
>
> --
> Pengutronix e.K.                | Johannes Zink                  |
> Steuerwalder Str. 21            | https://www.pengutronix.de/    |
> 31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
> Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |
>


-- 
Best Regards
Masahiro Yamada