On Sun, Mar 28, 2021 at 6:52 PM Mihai Moldovan <io...@ionic.de> wrote: > > If the user selects the very first entry in a page and performs a > search-up operation (e.g., via [/][a][Up Arrow]), nconf will never > terminate searching the page. > > The reason is that in this case, the starting point will be set to -1, > which is then translated into (n - 1) (i.e., the last entry of the > page) and finally the search begins. This continues to work fine until > the index reaches 0, at which point it will be decremented to -1, but > not checked against the starting point right away. Instead, it's > wrapped around to the bottom again, after which the starting point > check occurs... and naturally fails. > > We can easily avoid it by checking against the starting point directly > if the current index is -1 (which should be safe, since it's the only > magic value that can occur) and terminate the matching function. > > Amazingly, nobody seems to have been hit by this for 11 years - or at > the very least nobody bothered to debug and fix this. > > Signed-off-by: Mihai Moldovan <io...@ionic.de> > --- > v2: swap constant in comparison to right side, as requested by > Randy Dunlap <rdun...@infradead.org> > > scripts/kconfig/nconf.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c > index e0f965529166..db0dc46bc5ee 100644 > --- a/scripts/kconfig/nconf.c > +++ b/scripts/kconfig/nconf.c > @@ -515,6 +515,15 @@ static int get_mext_match(const char *match_str, match_f > flag) > --index; > else > ++index; > + /* > + * It's fine for index to become negative - think of an > + * initial value for match_start of 0 with a match direction > + * of up, eventually making it -1. > + * > + * Handle this as a special case. > + */ > + if ((index == -1) && (index == match_start)) > + return -1;
We know 'index' is -1 in the second comparison. So, you can also write like this: if (match_start == -1 && index == -1) return -1; But, it is not the correct fix, either. The root cause of the bug is match_start becoming -1. The following is the correct way to fix the bug without increasing the number of lines. diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c index e0f965529166..af814b39b876 100644 --- a/scripts/kconfig/nconf.c +++ b/scripts/kconfig/nconf.c @@ -504,8 +504,8 @@ static int get_mext_match(const char *match_str, match_f flag) else if (flag == FIND_NEXT_MATCH_UP) --match_start; + match_start = (match_start + items_num) % items_num; index = match_start; - index = (index + items_num) % items_num; while (true) { char *str = k_menu_items[index].str; if (strcasestr(str, match_str) != NULL) -- Best Regards Masahiro Yamada