[PATCH] riscv: lib: Fix ZBB strnlen reading past count boundary
Feng Jiang
jiangfeng at kylinos.cn
Sun Apr 12 23:51:39 PDT 2026
On 2026/4/13 13:02, Michael Neuling wrote:
>> 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 at 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 at 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),
Hi Mikey,
Thanks for the patch and for building the KUnit reproducer!
I've enhanced the existing string_test_strnlen() in lib/tests/string_kunit.c
to more comprehensively cover this issue. Since this covers both the over-read
fault and general logic correctness, I think we can simply improve the existing
test case rather than adding a new standalone string_test_strnlen_page_boundary().
Below is the diff for the enhanced test and the KUnit log showing it successfully
catching the failure (Oops) on RISC-V QEMU without your fix. Feel free to integrate
this into your patch series or let me know if you'd like me to submit it separately.
The Patch:
diff --git a/lib/tests/string_kunit.c b/lib/tests/string_kunit.c
index a8d430aa5466..27349a41bb10 100644
--- a/lib/tests/string_kunit.c
+++ b/lib/tests/string_kunit.c
@@ -155,6 +155,16 @@ static void string_test_strnlen(struct kunit *test)
for (size_t offset = 0; offset < STRING_TEST_MAX_OFFSET; offset++) {
for (size_t len = 0; len <= STRING_TEST_MAX_LEN; len++) {
+ /* Test strings without NUL terminator */
+ s = buf + buf_size - offset - len;
+ if (len > 0)
+ KUNIT_EXPECT_EQ(test, strnlen(s, len - 1), len - 1);
+ if (len > 1)
+ KUNIT_EXPECT_EQ(test, strnlen(s, len - 2), len - 2);
+
+ KUNIT_EXPECT_EQ(test, strnlen(s, len), len);
+
+ /* Test strings with NUL terminator */
s = buf + buf_size - 1 - offset - len;
s[len] = '\0';
@@ -169,6 +179,7 @@ static void string_test_strnlen(struct kunit *test)
KUNIT_EXPECT_EQ(test, strnlen(s, len + 2), len);
KUNIT_EXPECT_EQ(test, strnlen(s, len + 10), len);
+ /* Restore buffer */
s[len] = 'A';
Test Log (Catching the fault):
$ ./tools/testing/kunit/kunit.py run --arch=riscv --cross_compile=riscv64-linux-gnu- --kunitconfig=my_string.kunitconfig
[06:31:34] Configuring KUnit Kernel ...
[06:31:34] Building KUnit Kernel ...
Populating config with:
$ make ARCH=riscv O=.kunit olddefconfig CROSS_COMPILE=riscv64-linux-gnu-
Building with:
$ make all compile_commands.json scripts_gdb ARCH=riscv O=.kunit --jobs=6 CROSS_COMPILE=riscv64-linux-gnu-
[06:31:48] Starting KUnit Kernel (1/1)...
[06:31:48] ============================================================
Running tests with:
$ qemu-system-riscv64 -nodefaults -m 1024 -kernel .kunit/arch/riscv/boot/Image -append 'kunit.enable=1 console=ttyS0 kunit_shutdown=reboot' -no-reboot -nographic -accel kvm -accel hvf -accel tcg -serial stdio -machine virt -cpu rv64 -bios /usr/share/qemu/opensbi-riscv64-generic-fw_dynamic.bin
[06:31:49] =================== string (28 subtests) ===================
[06:31:50] [PASSED] string_test_memset16
[06:31:51] [PASSED] string_test_memset32
[06:31:51] [PASSED] string_test_memset64
[06:31:51] [PASSED] string_test_strlen
[06:31:51] Unable to handle kernel paging request at virtual address ff20000000026000
[06:31:51] Current kunit_try_catch pgtable: 4K pagesize, 57-bit VAs, pgdp=0x0000000080eac000
[06:31:51] [ff20000000026000] pgd=0000000020400c01, p4d=0000000020415001, pud=0000000020415401, pmd=0000000020415801, pte=0000000000000000
[06:31:51] Oops [#1]
[06:31:51] CPU: 0 UID: 0 PID: 25 Comm: kunit_try_catch Tainted: G N 6.19.0-rc8-00122-g17585ffa9746-dirty #301 NONE
[06:31:51] Tainted: [N]=TEST
[06:31:51] Hardware name: riscv-virtio,qemu (DT)
[06:31:51] epc : strnlen_zbb+0x40/0x78
[06:31:51] ra : string_test_strnlen+0x156/0x420
[06:31:51] epc : ffffffff802663a8 ra : ffffffff801a53d6 sp : ff200000000abd30
[06:31:51] gp : ffffffff80ca2a58 tp : ff60000001133340 t0 : ff20000000025ff8
[06:31:51] t1 : 0000000000000008 t2 : ff20000000026000 s0 : ff200000000abdd0
[06:31:51] s1 : ff2000000000b928 a0 : 0000000000000001 a1 : 0000000000000001
[06:31:51] a2 : 0000000000000000 a3 : ff20000000026000 a4 : 0000000000000000
[06:31:51] a5 : 00000000000000a1 a6 : ffffffff808a1e68 a7 : 0000000000000001
[06:31:51] s2 : ffffffff80890288 s3 : 0000000000000001 s4 : 0000000000000000
[06:31:51] s5 : ff20000000025ffe s6 : 0000000000000001 s7 : ffffffffffffffff
[06:31:51] s8 : ff20000000025fff s9 : ff20000000025fff s10: ff20000000026000
[06:31:51] s11: 0000000000000081 t3 : ffffffffffffffff t4 : ff20000000026000
[06:31:51] t5 : 0000000000000d25 t6 : ff60000001055000
[06:31:51] status: 0000000200000120 badaddr: ff20000000026000 cause: 000000000000000d
[06:31:51] [<ffffffff802663a8>] strnlen_zbb+0x40/0x78
[06:31:51] [<ffffffff8019bed4>] kunit_try_run_case+0x58/0x144
[06:31:51] [<ffffffff8019dd7e>] kunit_generic_run_threadfn_adapter+0x1a/0x34
[06:31:51] [<ffffffff80033fda>] kthread+0xa6/0x144
[06:31:51] [<ffffffff8000a5ba>] ret_from_fork_kernel+0xe/0xf0
[06:31:51] [<ffffffff8026e13a>] ret_from_fork_kernel_asm+0x16/0x18
[06:31:51] Code: 6013 5513 0033 5533 0ab5 6a63 03c5 8393 0082 5e7d (b303) 0082
[06:31:51] ---[ end trace 0000000000000000 ]---
[06:31:51] # string_test_strnlen: try faulted: last line seen lib/tests/string_kunit.c:161
[06:31:51] # string_test_strnlen: internal error occurred preventing test case from running: -4
[06:31:51] [FAILED] string_test_strnlen
[06:31:51] [PASSED] string_test_strchr
[06:31:51] [PASSED] string_test_strnchr
[06:31:51] [PASSED] string_test_strrchr
[06:31:51] [PASSED] string_test_strspn
[06:31:51] [PASSED] string_test_strcmp
[06:31:51] [PASSED] string_test_strcmp_long_strings
[06:31:51] [PASSED] string_test_strncmp
[06:31:51] [PASSED] string_test_strncmp_long_strings
[06:31:51] [PASSED] string_test_strcasecmp
[06:31:51] [PASSED] string_test_strcasecmp_long_strings
[06:31:51] [PASSED] string_test_strncasecmp
[06:31:51] [PASSED] string_test_strncasecmp_long_strings
[06:31:51] [PASSED] string_test_strscpy
[06:31:51] [PASSED] string_test_strcat
[06:31:51] [PASSED] string_test_strncat
[06:31:51] [PASSED] string_test_strlcat
[06:31:51] [PASSED] string_test_strtomem
[06:31:51] [PASSED] string_test_memtostr
[06:31:51] [PASSED] string_test_strends
[06:31:51] [PASSED] string_bench_strlen
[06:31:51] [PASSED] string_bench_strnlen
[06:31:51] [PASSED] string_bench_strchr
[06:31:51] [PASSED] string_bench_strrchr
[06:31:51] # module: string_kunit
[06:31:51] # string: pass:27 fail:1 skip:0 total:28
[06:31:51] # Totals: pass:27 fail:1 skip:0 total:28
[06:31:51] ===================== [FAILED] string ======================
[06:31:51] ============================================================
[06:31:51] Testing complete. Ran 28 tests: passed: 27, failed: 1
[06:31:52] Elapsed time: 17.644s total, 0.001s configuring, 14.419s building, 3.141s running
Thanks again!
--
With Best Regards,
Feng Jiang
More information about the linux-riscv
mailing list