[PATCH bpf-next 1/2] bpf: add bpf_strcat,bpf_strncat kfunc

[PATCH bpf-next 1/2] bpf: add bpf_strcat,bpf_strncat kfunc

[Rong Tao]

From: Rong Tao <rongtao@cestc.cn>

Add string concatenation kfuncs, prototype:

  int bpf_strcat(char *dst__ign, u32 dst__sz, const char *src__ign);
  int bpf_strncat(char *dst__ign, u32 dst__sz, const char *src__ign, u32 len);

This differs from the glibc library functions strcat and strncat, which,
for safety reasons, require the size of the target string's memory space
as a parameter.

Signed-off-by: Rong Tao <rongtao@cestc.cn>
---
 kernel/bpf/helpers.c | 91 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index c18f1e16edee..e99b64313682 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -4195,6 +4195,95 @@ __bpf_kfunc int bpf_strncasestr(const char *s1__ign, const char *s2__ign,
 	return __bpf_strnstr(s1__ign, s2__ign, len, true);
 }
 
+static int __bpf_strncat(char *dst, u32 dsz, const char *src, u32 sz)
+{
+	int dlen, slen, space, copied;
+	char cs = '?';
+
+	if (!copy_from_kernel_nofault_allowed(dst, 1) ||
+	    !copy_from_kernel_nofault_allowed(src, 1)) {
+		return -ERANGE;
+	}
+
+	dlen = bpf_strnlen(dst, XATTR_SIZE_MAX);
+	if (dlen < 0)
+		return dlen;
+	slen = bpf_strnlen(src, XATTR_SIZE_MAX);
+	if (slen < 0)
+		return slen;
+
+	if (dlen >= dsz || sz == 0 || dsz == 0)
+		return -EINVAL;
+
+	space = dsz - dlen;
+	if (space <= 1 || space < min(slen, sz) + 1)
+		return -E2BIG;
+
+	for (copied = 0; copied < space - 1 && copied < sz; copied++) {
+		__get_kernel_nofault(&cs, src, char, err_out);
+		if (cs == '\0')
+			break;
+
+		__put_kernel_nofault(dst + dlen + copied, &cs, char, err_out);
+
+		src++;
+	}
+	cs = '\0';
+	__put_kernel_nofault(dst + dlen + copied, &cs, char, err_out);
+
+	__get_kernel_nofault(&cs, src, char, err_out);
+	if (cs != '\0' && sz > copied)
+		return -E2BIG;
+
+	return copied;
+err_out:
+	return -EFAULT;
+}
+
+/**
+ * bpf_strcat - Append non-null bytes from a source string, and null-terminate
+ *              the result
+ * @dst__ign: Destination string.
+ * @dst__sz: Maximum bytes of @dst__ign, includes the trailing NUL.
+ * @src__ign: Source string.
+ *
+ * Return:
+ * * >=0      - Length of the concatenated string.
+ *
+ * * %-EINVAL - String @dst__ign is invalid.
+ * * %-EFAULT - Cannot read or write one of the strings.
+ * * %-E2BIG  - String @src__ign is too large or the remaining space in
+ *              @dst__ign is too small.
+ * * %-ERANGE - One of the strings is outside of kernel address space
+ */
+__bpf_kfunc int bpf_strcat(char *dst__ign, u32 dst__sz, const char *src__ign)
+{
+	return __bpf_strncat(dst__ign, dst__sz, src__ign, XATTR_SIZE_MAX);
+}
+
+/**
+ * bpf_strncat - Append non-null bytes from a source string, and null-terminate
+ *               the result
+ * @dst__ign: Destination string.
+ * @dst__sz: Maximum bytes of @dst__ign, includes the trailing NUL.
+ * @src__ign: Source string.
+ * @len: the maximum number of characters to concatenate
+ *
+ * Return:
+ * * >=0      - Length of the concatenated string.
+ *
+ * * %-EINVAL - String @dst__ign is invalid.
+ * * %-EFAULT - Cannot read or write one of the strings.
+ * * %-E2BIG  - String @src__ign is too large or the remaining space in
+ *              @dst__ign is too small.
+ * * %-ERANGE - One of the strings is outside of kernel address space
+ */
+__bpf_kfunc int bpf_strncat(char *dst__ign, u32 dst__sz, const char *src__ign,
+			    u32 len)
+{
+	return __bpf_strncat(dst__ign, dst__sz, src__ign, len);
+}
+
 #ifdef CONFIG_KEYS
 /**
  * bpf_lookup_user_key - lookup a key by its serial
@@ -4958,6 +5047,8 @@ BTF_ID_FLAGS(func, bpf_strstr);
 BTF_ID_FLAGS(func, bpf_strcasestr);
 BTF_ID_FLAGS(func, bpf_strnstr);
 BTF_ID_FLAGS(func, bpf_strncasestr);
+BTF_ID_FLAGS(func, bpf_strcat);
+BTF_ID_FLAGS(func, bpf_strncat);
 #if defined(CONFIG_BPF_LSM) && defined(CONFIG_CGROUPS)
 BTF_ID_FLAGS(func, bpf_cgroup_read_xattr, KF_RCU)
 #endif
-- 
2.55.0

Re: [PATCH bpf-next 1/2] bpf: add bpf_strcat,bpf_strncat kfunc

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] Arbitrary Kernel Memory Write via Verifier Bypass
- [High] Missing page fault protection leading to sleeping in atomic context
- [High] Time-Of-Check to Time-Of-Use (TOCTOU) race leading to unbounded loop and soft lockup
- [Medium] Unbounded out-of-bounds read on destination buffer
- [Medium] Unbounded out-of-bounds read on source buffer
--

--- Patch [1]: [PATCH bpf-next 1/2] bpf: add bpf_strcat,bpf_strncat kfunc ---
commit a7b2e5fa67dde4d93faf939b8283a2aa59b8ca2d
Author: Rong Tao <rongtao@cestc.cn>

bpf: add bpf_strcat,bpf_strncat kfunc

This commit introduces bpf_strcat and bpf_strncat kfuncs to allow string
concatenation in BPF programs. It requires the destination buffer size as a
parameter for safety.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index c18f1e16edee4..e99b643136822 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
[ ... ]
> @@ -4195,6 +4195,95 @@ __bpf_kfunc int bpf_strncasestr(const char *s1__ign, const char *s2__ign,
>  	return __bpf_strnstr(s1__ign, s2__ign, len, true);
>  }
>  
> +static int __bpf_strncat(char *dst, u32 dsz, const char *src, u32 sz)
> +{
> +	int dlen, slen, space, copied;
> +	char cs = '?';
> +
> +	if (!copy_from_kernel_nofault_allowed(dst, 1) ||
> +	    !copy_from_kernel_nofault_allowed(src, 1)) {
> +		return -ERANGE;
> +	}
> +
> +	dlen = bpf_strnlen(dst, XATTR_SIZE_MAX);

[Severity: Medium]
Does this code perform an unbounded read on the destination buffer? The limit
should be bounded by the provided size dsz, rather than statically set to
XATTR_SIZE_MAX.

> +	if (dlen < 0)
> +		return dlen;
> +	slen = bpf_strnlen(src, XATTR_SIZE_MAX);

[Severity: Medium]
Does this scan read out-of-bounds on the source buffer? The limit could be
bounded by the requested copy length sz rather than unconditionally reading
up to XATTR_SIZE_MAX.

> +	if (slen < 0)
> +		return slen;
> +
> +	if (dlen >= dsz || sz == 0 || dsz == 0)
> +		return -EINVAL;
> +
> +	space = dsz - dlen;
> +	if (space <= 1 || space < min(slen, sz) + 1)
> +		return -E2BIG;
> +
> +	for (copied = 0; copied < space - 1 && copied < sz; copied++) {

[Severity: High]
Does this code introduce a Time-Of-Check to Time-Of-Use race? The loop bounds
omit checking copied < slen, relying entirely on encountering a null
terminator.

If a concurrent thread modifies the source buffer to remove the null terminator
after bpf_strnlen() executes, could this loop run out of bounds and lead to a
soft lockup?

> +		__get_kernel_nofault(&cs, src, char, err_out);

[Severity: High]
Does this sequence lack page fault protection? Unlike other string helpers,
__bpf_strncat() fails to wrap its memory accesses in guard(pagefault)().

If a BPF program executing in a non-sleepable context calls this on an unmapped
address, it seems the generic page fault handler will attempt to take sleepable
locks.

> +		if (cs == '\0')
> +			break;
> +
> +		__put_kernel_nofault(dst + dlen + copied, &cs, char, err_out);

[Severity: Critical]
Can this write to arbitrary kernel memory? The function writes to the unchecked
destination pointer.

Because the bpf_strcat() kfunc signature uses the __ign suffix for dst__ign,
verifier safety checks are bypassed. Any user capable of loading BPF programs
could potentially use this to write user-controlled bytes to arbitrary kernel
memory.

> +
> +		src++;
> +	}
[ ... ]
> +__bpf_kfunc int bpf_strcat(char *dst__ign, u32 dst__sz, const char *src__ign)

[Severity: Critical]
Does the dst__ign argument bypass verifier checks? The __ign suffix explicitly
instructs the BPF verifier to ignore this argument, which removes all memory
safety and bounds checks for the destination pointer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784021302.git.rongtao@cestc.cn?part=1

Re: [PATCH bpf-next 1/2] bpf: add bpf_strcat,bpf_strncat kfunc

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2834 bytes --]

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index c18f1e16edee4..e99b643136822 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -4195,6 +4195,95 @@ __bpf_kfunc int bpf_strncasestr(const char *s1__ign, const char *s2__ign,
> +static int __bpf_strncat(char *dst, u32 dsz, const char *src, u32 sz)
> +{
> +	int dlen, slen, space, copied;

[ ... ]

> +	for (copied = 0; copied < space - 1 && copied < sz; copied++) {
> +		__get_kernel_nofault(&cs, src, char, err_out);
> +		if (cs == '\0')
> +			break;
> +
> +		__put_kernel_nofault(dst + dlen + copied, &cs, char, err_out);
> +
> +		src++;
> +	}
> +	cs = '\0';
> +	__put_kernel_nofault(dst + dlen + copied, &cs, char, err_out);
> +
> +	__get_kernel_nofault(&cs, src, char, err_out);
> +	if (cs != '\0' && sz > copied)
> +		return -E2BIG;
> +
> +	return copied;

Here __bpf_strncat() returns copied, which counts only the bytes
appended from src.  It does not include dlen, the pre-existing length of
dst.

> +err_out:
> +	return -EFAULT;
> +}
> +
> +/**
> + * bpf_strcat - Append non-null bytes from a source string, and null-terminate
> + *              the result
> + * @dst__ign: Destination string.
> + * @dst__sz: Maximum bytes of @dst__ign, includes the trailing NUL.
> + * @src__ign: Source string.
> + *
> + * Return:
> + * * >=0      - Length of the concatenated string.

The kernel-doc for both bpf_strcat() and bpf_strncat() describes the
success return as ">=0 - Length of the concatenated string."

Since __bpf_strncat() returns copied rather than dlen + copied, does the
returned value actually describe the number of bytes appended rather than
the length of the resulting string?

Should either the return be dlen + copied, or the documentation say
"number of bytes appended"?

> + */
> +__bpf_kfunc int bpf_strcat(char *dst__ign, u32 dst__sz, const char *src__ign)
> +{
> +	return __bpf_strncat(dst__ign, dst__sz, src__ign, XATTR_SIZE_MAX);
> +}
> +
> +/**
> + * bpf_strncat - Append non-null bytes from a source string, and null-terminate
> + *               the result
> + * @dst__ign: Destination string.
> + * @dst__sz: Maximum bytes of @dst__ign, includes the trailing NUL.
> + * @src__ign: Source string.
> + * @len: the maximum number of characters to concatenate
> + *
> + * Return:
> + * * >=0      - Length of the concatenated string.

The same "Length of the concatenated string" wording applies here.

> + */
> +__bpf_kfunc int bpf_strncat(char *dst__ign, u32 dst__sz, const char *src__ign,
> +			    u32 len)
> +{
> +	return __bpf_strncat(dst__ign, dst__sz, src__ign, len);
> +}


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/29323723211

[PATCH bpf-next 2/2] selftests/bpf: Test bpf_strcat,bpf_strncat kfuncs

[Rong Tao]

From: Rong Tao <rongtao@cestc.cn>

Add tests for new kfuncs bpf_strcat() and bpf_strncat().

Signed-off-by: Rong Tao <rongtao@cestc.cn>
---
 .../selftests/bpf/prog_tests/string_kfuncs.c        |  2 ++
 .../selftests/bpf/progs/string_kfuncs_failure1.c    | 13 +++++++++++++
 .../selftests/bpf/progs/string_kfuncs_failure2.c    |  2 ++
 .../selftests/bpf/progs/string_kfuncs_success.c     |  3 +++
 4 files changed, 20 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/string_kfuncs.c b/tools/testing/selftests/bpf/prog_tests/string_kfuncs.c
index 300032a19445..460567ef622a 100644
--- a/tools/testing/selftests/bpf/prog_tests/string_kfuncs.c
+++ b/tools/testing/selftests/bpf/prog_tests/string_kfuncs.c
@@ -24,6 +24,8 @@ static const char * const test_cases[] = {
 	"strcasestr",
 	"strnstr",
 	"strncasestr",
+	"strcat",
+	"strncat",
 };
 
 void run_too_long_tests(void)
diff --git a/tools/testing/selftests/bpf/progs/string_kfuncs_failure1.c b/tools/testing/selftests/bpf/progs/string_kfuncs_failure1.c
index bddc4e8579d2..29bc7eabbb6a 100644
--- a/tools/testing/selftests/bpf/progs/string_kfuncs_failure1.c
+++ b/tools/testing/selftests/bpf/progs/string_kfuncs_failure1.c
@@ -8,6 +8,7 @@
 
 char *user_ptr = (char *)1;
 char *invalid_kern_ptr = (char *)-1;
+char kern_buf[32] = { "hello" };
 
 /*
  * When passing userspace pointers, the error code differs based on arch:
@@ -53,6 +54,10 @@ SEC("syscall")  __retval(USER_PTR_ERR)int test_strnstr_null1(void *ctx) { return
 SEC("syscall")  __retval(USER_PTR_ERR)int test_strnstr_null2(void *ctx) { return bpf_strnstr("hello", NULL, 1); }
 SEC("syscall")  __retval(USER_PTR_ERR)int test_strncasestr_null1(void *ctx) { return bpf_strncasestr(NULL, "hello", 1); }
 SEC("syscall")  __retval(USER_PTR_ERR)int test_strncasestr_null2(void *ctx) { return bpf_strncasestr("hello", NULL, 1); }
+SEC("syscall")  __retval(USER_PTR_ERR)int test_strcat_null1(void *ctx) { return bpf_strcat(NULL, 6, "hello"); }
+SEC("syscall")  __retval(USER_PTR_ERR)int test_strcat_null2(void *ctx) { return bpf_strcat(kern_buf, sizeof(kern_buf), NULL); }
+SEC("syscall")  __retval(USER_PTR_ERR)int test_strncat_null1(void *ctx) { return bpf_strncat(NULL, 6, "hello", 2); }
+SEC("syscall")  __retval(USER_PTR_ERR)int test_strncat_null2(void *ctx) { return bpf_strncat(kern_buf, sizeof(kern_buf), NULL, 2); }
 
 /* Passing userspace ptr to string kfuncs */
 SEC("syscall") __retval(USER_PTR_ERR) int test_strcmp_user_ptr1(void *ctx) { return bpf_strcmp(user_ptr, "hello"); }
@@ -79,6 +84,10 @@ SEC("syscall") __retval(USER_PTR_ERR) int test_strnstr_user_ptr1(void *ctx) { re
 SEC("syscall") __retval(USER_PTR_ERR) int test_strnstr_user_ptr2(void *ctx) { return bpf_strnstr("hello", user_ptr, 1); }
 SEC("syscall") __retval(USER_PTR_ERR) int test_strncasestr_user_ptr1(void *ctx) { return bpf_strncasestr(user_ptr, "hello", 1); }
 SEC("syscall") __retval(USER_PTR_ERR) int test_strncasestr_user_ptr2(void *ctx) { return bpf_strncasestr("hello", user_ptr, 1); }
+SEC("syscall") __retval(USER_PTR_ERR) int test_strcat_user_ptr1(void *ctx) { return bpf_strcat(user_ptr, 1, "hello"); }
+SEC("syscall") __retval(USER_PTR_ERR) int test_strcat_user_ptr2(void *ctx) { return bpf_strcat(kern_buf, sizeof(kern_buf), user_ptr); }
+SEC("syscall") __retval(USER_PTR_ERR) int test_strncat_user_ptr1(void *ctx) { return bpf_strncat(user_ptr, 1, "hello", 2); }
+SEC("syscall") __retval(USER_PTR_ERR) int test_strncat_user_ptr2(void *ctx) { return bpf_strncat(kern_buf, sizeof(kern_buf), user_ptr, 1); }
 
 #endif /* __TARGET_ARCH_s390 */
 
@@ -107,5 +116,9 @@ SEC("syscall") __retval(-EFAULT) int test_strnstr_pagefault1(void *ctx) { return
 SEC("syscall") __retval(-EFAULT) int test_strnstr_pagefault2(void *ctx) { return bpf_strnstr("hello", invalid_kern_ptr, 1); }
 SEC("syscall") __retval(-EFAULT) int test_strncasestr_pagefault1(void *ctx) { return bpf_strncasestr(invalid_kern_ptr, "hello", 1); }
 SEC("syscall") __retval(-EFAULT) int test_strncasestr_pagefault2(void *ctx) { return bpf_strncasestr("hello", invalid_kern_ptr, 1); }
+SEC("syscall") __retval(-EFAULT) int test_strcat_pagefault1(void *ctx) { return bpf_strcat(invalid_kern_ptr, 1, "hello"); }
+SEC("syscall") __retval(-EFAULT) int test_strcat_pagefault2(void *ctx) { return bpf_strcat(kern_buf, sizeof(kern_buf), invalid_kern_ptr); }
+SEC("syscall") __retval(-EFAULT) int test_strncat_pagefault1(void *ctx) { return bpf_strncat(invalid_kern_ptr, 1, "hello", 2); }
+SEC("syscall") __retval(-EFAULT) int test_strncat_pagefault2(void *ctx) { return bpf_strncat(kern_buf, sizeof(kern_buf), invalid_kern_ptr, 2); }
 
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/string_kfuncs_failure2.c b/tools/testing/selftests/bpf/progs/string_kfuncs_failure2.c
index 412c53b87b18..38f0a5f326b5 100644
--- a/tools/testing/selftests/bpf/progs/string_kfuncs_failure2.c
+++ b/tools/testing/selftests/bpf/progs/string_kfuncs_failure2.c
@@ -23,5 +23,7 @@ SEC("syscall") int test_strstr_too_long(void *ctx) { return bpf_strstr(long_str,
 SEC("syscall") int test_strcasestr_too_long(void *ctx) { return bpf_strcasestr(long_str, "hello"); }
 SEC("syscall") int test_strnstr_too_long(void *ctx) { return bpf_strnstr(long_str, "hello", sizeof(long_str)); }
 SEC("syscall") int test_strncasestr_too_long(void *ctx) { return bpf_strncasestr(long_str, "hello", sizeof(long_str)); }
+SEC("syscall") int test_strcat_too_long(void *ctx) { return bpf_strcat(long_str, sizeof(long_str), "hello"); }
+SEC("syscall") int test_strncat_too_long(void *ctx) { return bpf_strncat(long_str, sizeof(long_str), "hello", 3); }
 
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/string_kfuncs_success.c b/tools/testing/selftests/bpf/progs/string_kfuncs_success.c
index f65b1226a81a..b45e559a3178 100644
--- a/tools/testing/selftests/bpf/progs/string_kfuncs_success.c
+++ b/tools/testing/selftests/bpf/progs/string_kfuncs_success.c
@@ -6,6 +6,7 @@
 #include "errno.h"
 
 char str[] = "hello world";
+char buf[32] = "hello";
 
 #define __test(retval) SEC("syscall") __success __retval(retval)
 
@@ -59,5 +60,7 @@ __test(-ENOENT) int test_strncasestr_notfound1(void *ctx) { return bpf_strncases
 __test(-ENOENT) int test_strncasestr_notfound2(void *ctx) { return bpf_strncasestr(str, "hello", 4); }
 __test(-ENOENT) int test_strncasestr_notfound3(void *ctx) { return bpf_strncasestr("", "a", 0); }
 __test(0) int test_strncasestr_empty(void *ctx) { return bpf_strncasestr(str, "", 1); }
+__test(5) int test_strcat_success(void *ctx) { return bpf_strcat(buf, sizeof(buf), "world"); }
+__test(3) int test_strncat_success(void *ctx) { return bpf_strncat(buf, sizeof(buf), "world", 3); }
 
 char _license[] SEC("license") = "GPL";
-- 
2.55.0

Re: [PATCH bpf-next 2/2] selftests/bpf: Test bpf_strcat,bpf_strncat kfuncs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] The `bpf_strcat` and `bpf_strncat` kfuncs use the `__ign` suffix for their destination pointer arguments, bypassing the BPF verifier's safety checks and creating an arbitrary kernel memory write vulnerability.
- [Medium] `__bpf_strncat` performs out-of-bounds reads on the source string by ignoring the user-provided limit during length calculation and unconditionally fetching an extra byte after the copy loop.
- [Low] The API documentation for `bpf_strcat` and `bpf_strncat` incorrectly states they return the total length of the concatenated string.
--

--- Patch [2]: [PATCH bpf-next 2/2] selftests/bpf: Test bpf_strcat,bpf_strncat kfuncs ---
commit a7b2e5fa67dde4d93faf939b8283a2aa59b8ca2d
Author: Rong Tao <rongtao@cestc.cn>

bpf: add bpf_strcat,bpf_strncat kfunc

Add string concatenation kfuncs bpf_strcat and bpf_strncat. This allows
BPF programs to concatenate strings, differing from glibc by requiring
the target string's memory size for safety.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index c18f1e16edee4..e99b643136822 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c

[ ... ]

> @@ -4195,6 +4195,95 @@ __bpf_kfunc int bpf_strncasestr(const char *s1__ign, const char *s2__ign,
>  	return __bpf_strnstr(s1__ign, s2__ign, len, true);
>  }
>  
> +static int __bpf_strncat(char *dst, u32 dsz, const char *src, u32 sz)
> +{
> +	int dlen, slen, space, copied;
> +	char cs = '?';
> +
> +	if (!copy_from_kernel_nofault_allowed(dst, 1) ||
> +	    !copy_from_kernel_nofault_allowed(src, 1)) {
> +		return -ERANGE;
> +	}
> +
> +	dlen = bpf_strnlen(dst, XATTR_SIZE_MAX);
> +	if (dlen < 0)
> +		return dlen;
> +	slen = bpf_strnlen(src, XATTR_SIZE_MAX);

[Severity: Medium]
Does this calculate the length of the source string past the user-provided
limit sz?

If src is exactly sz bytes long and not null-terminated, won't this read
out of bounds up to XATTR_SIZE_MAX and potentially return -EFAULT?

> +	if (slen < 0)
> +		return slen;
> +
> +	if (dlen >= dsz || sz == 0 || dsz == 0)
> +		return -EINVAL;
> +
> +	space = dsz - dlen;
> +	if (space <= 1 || space < min(slen, sz) + 1)
> +		return -E2BIG;
> +
> +	for (copied = 0; copied < space - 1 && copied < sz; copied++) {
> +		__get_kernel_nofault(&cs, src, char, err_out);
> +		if (cs == '\0')
> +			break;
> +
> +		__put_kernel_nofault(dst + dlen + copied, &cs, char, err_out);
> +
> +		src++;
> +	}
> +	cs = '\0';
> +	__put_kernel_nofault(dst + dlen + copied, &cs, char, err_out);
> +
> +	__get_kernel_nofault(&cs, src, char, err_out);

[Severity: Medium]
If the copy loop ends because copied == sz, does this unconditional read
fetch an extra byte from the advanced src pointer?

Could this cause an out-of-bounds read and a false -EFAULT failure?

> +	if (cs != '\0' && sz > copied)
> +		return -E2BIG;
> +
> +	return copied;
> +err_out:
> +	return -EFAULT;
> +}
> +
> +/**
> + * bpf_strcat - Append non-null bytes from a source string, and null-terminate
> + *              the result
> + * @dst__ign: Destination string.
> + * @dst__sz: Maximum bytes of @dst__ign, includes the trailing NUL.
> + * @src__ign: Source string.
> + *
> + * Return:
> + * * >=0      - Length of the concatenated string.

[Severity: Low]
Is this documentation misleading?

It claims to return the length of the concatenated string, but __bpf_strncat
returns copied, which is the number of newly appended characters.

> + *
> + * * %-EINVAL - String @dst__ign is invalid.
> + * * %-EFAULT - Cannot read or write one of the strings.
> + * * %-E2BIG  - String @src__ign is too large or the remaining space in
> + *              @dst__ign is too small.
> + * * %-ERANGE - One of the strings is outside of kernel address space
> + */
> +__bpf_kfunc int bpf_strcat(char *dst__ign, u32 dst__sz, const char *src__ign)

[Severity: Critical]
Does the __ign suffix on dst__ign bypass the BPF verifier's safety checks?

Since is_kfunc_arg_ignore() in kernel/bpf/verifier.c skips validation for
arguments with the __ign suffix, can a BPF program pass an arbitrary kernel
address (like modprobe_path) instead of a validated PTR_TO_MEM, allowing an
arbitrary kernel memory write?

> +{
> +	return __bpf_strncat(dst__ign, dst__sz, src__ign, XATTR_SIZE_MAX);
> +}
> +
> +/**
> + * bpf_strncat - Append non-null bytes from a source string, and null-terminate
> + *               the result
> + * @dst__ign: Destination string.
> + * @dst__sz: Maximum bytes of @dst__ign, includes the trailing NUL.
> + * @src__ign: Source string.
> + * @len: the maximum number of characters to concatenate
> + *
> + * Return:
> + * * >=0      - Length of the concatenated string.
> + *
> + * * %-EINVAL - String @dst__ign is invalid.
> + * * %-EFAULT - Cannot read or write one of the strings.
> + * * %-E2BIG  - String @src__ign is too large or the remaining space in
> + *              @dst__ign is too small.
> + * * %-ERANGE - One of the strings is outside of kernel address space
> + */
> +__bpf_kfunc int bpf_strncat(char *dst__ign, u32 dst__sz, const char *src__ign,
> +			    u32 len)

[Severity: Critical]
Does the dst__ign parameter here also bypass verifier safety checks for the
same reason, allowing arbitrary memory writes?

> +{
> +	return __bpf_strncat(dst__ign, dst__sz, src__ign, len);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784021302.git.rongtao@cestc.cn?part=2