Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Mihai Moldovan <ionic@ionic.de>
To:     Masahiro Yamada <masahiroy@kernel.org>
Cc:     linux-kernel@vger.kernel.org
Subject: [PATCH v2] kconfig: nconf: stop endless search-up loops
Date:   Sun, 28 Mar 2021 11:52:27 +0200
Message-Id: <20210328095227.24323-1-ionic@ionic.de>
In-Reply-To: <20210327120155.500-1-ionic@ionic.de>
References: <20210327120155.500-1-ionic@ionic.de>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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 <ionic@ionic.de>
---
v2: swap constant in comparison to right side, as requested by
    Randy Dunlap <rdunlap@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;
 		index = (index + items_num) % items_num;
 		if (index == match_start)
 			return -1;
-- 
2.30.1