Re: [PATCH] riscv: lib: Fix ZBB strnlen reading past count boundary

[Michael Neuling <mikey@neuling.org>]

> Thanks for catching and fixing this! Your analysis is spot on—that
> "load-before-check" logic was indeed an oversight on my part, especially
> regarding the page boundary edge case.

No worries.

> The test case you provided is extremely helpful. Since you've already
> built this reproducer, would you be interested in helping to improve
> the KUnit test string_test_strnlen() in lib/tests/string_kunit.c as well?
> Currently, it mainly tests strings with NUL terminators and lacks coverage
> for these kinds of non-terminated boundary scenarios.

The below is from Claude. I gave it a test under qemu riscv with and without 
the patch and it seems to catch the failure. Feel free to use it as you see fit.

  [   19.042129] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
  [   19.043133] Oops [#1]
  [   66.197273]     ok 5 string_test_strnlen
  [   66.197855] Unable to handle kernel paging request at virtual address ff20000000096000
  [   66.198980] Oops [#2]
  [   66.199867]  ra : string_test_strnlen_page_boundary+0xba/0x244
  [   66.204025]     # string_test_strnlen_page_boundary: try faulted: last line seen lib/tests/string_kunit.c:195
  [   66.204391]     # string_test_strnlen_page_boundary: internal error occurred preventing test case from running: -4
  [   66.205133]     not ok 6 string_test_strnlen_page_boundary
  [   66.227546] # string: pass:24 fail:1 skip:4 total:29
  [   66.227879] not ok 74 string

Mikey

From b4933270b53e3acccea707b6dced352ee525828f Mon Sep 17 00:00:00 2001
From: Michael Neuling <mikey@neuling.org>
Date: Mon, 13 Apr 2026 04:17:56 +0000
Subject: [PATCH] lib/string_kunit: add strnlen page boundary test

Add a kunit test that exercises strnlen with count reaching exactly to a
page boundary and no NUL terminator in the buffer.  This catches
implementations that speculatively read past the count boundary (e.g. a
word-at-a-time loop that loads before checking the limit).

The test uses vmap of a single page so the next page is an unmapped
guard page.  A buggy strnlen that reads past count will fault.

Three cases are tested:
- No NUL in buffer, count 1-128 reaching page end (the primary trigger)
- NUL present near the page boundary (correctness check)
- count=0 with pointer at the page boundary (should not read at all)

Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Claude Opus 4.6 (1M context)
---
 lib/tests/string_kunit.c | 47 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/lib/tests/string_kunit.c b/lib/tests/string_kunit.c
index 0819ace5b0..917ff8edef 100644
--- a/lib/tests/string_kunit.c
+++ b/lib/tests/string_kunit.c
@@ -176,6 +176,52 @@ static void string_test_strnlen(struct kunit *test)
 	vfree(buf);
 }
 
+/*
+ * Test strnlen with count reaching a page boundary and no NUL terminator
+ * in the buffer.  A buggy implementation that reads past the count boundary
+ * (e.g. a word-at-a-time loop that loads before checking) will fault on
+ * the unmapped guard page that vmap places after the mapping.
+ */
+static void string_test_strnlen_page_boundary(struct kunit *test)
+{
+	struct page *page;
+	char *buf;
+	size_t count;
+
+	page = alloc_page(GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, page);
+
+	buf = vmap(&page, 1, VM_MAP, PAGE_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, buf);
+
+	memset(buf, 'A', PAGE_SIZE);
+
+	/* Count reaches exactly to the page boundary, no NUL in buffer. */
+	for (count = 1; count <= 128; count++) {
+		char *s = buf + PAGE_SIZE - count;
+
+		KUNIT_EXPECT_EQ_MSG(test, strnlen(s, count), count,
+			"count:%zu offset_from_end:%zu", count, count);
+	}
+
+	/* Also test with NUL present within the buffer near the boundary. */
+	for (count = 2; count <= 128; count++) {
+		char *s = buf + PAGE_SIZE - count;
+		size_t nul_pos = count / 2;
+
+		s[nul_pos] = '\0';
+		KUNIT_EXPECT_EQ_MSG(test, strnlen(s, count), nul_pos,
+			"count:%zu nul_pos:%zu", count, nul_pos);
+		s[nul_pos] = 'A';
+	}
+
+	/* count = 0 should not read at all, even at the page boundary. */
+	KUNIT_EXPECT_EQ(test, strnlen(buf + PAGE_SIZE, 0), (size_t)0);
+
+	vunmap(buf);
+	__free_page(page);
+}
+
 static void string_test_strchr(struct kunit *test)
 {
 	const char *test_string = "abcdefghijkl";
@@ -887,6 +933,7 @@ static struct kunit_case string_test_cases[] = {
 	KUNIT_CASE(string_test_memset64),
 	KUNIT_CASE(string_test_strlen),
 	KUNIT_CASE(string_test_strnlen),
+	KUNIT_CASE(string_test_strnlen_page_boundary),
 	KUNIT_CASE(string_test_strchr),
 	KUNIT_CASE(string_test_strnchr),
 	KUNIT_CASE(string_test_strrchr),
-- 
2.43.0