On Mon, Jul 27, 2026 at 7:50 PM Rong Tao <[email protected]> wrote:
>
>
> On 7/28/26 06:56, Andrii Nakryiko wrote:
> > On Tue, Jul 21, 2026 at 9:42 PM Rong Tao <[email protected]> wrote:
> >> From: Rong Tao <[email protected]>
> >>
> >> Add string concatenation kfuncs, prototype:
> >>
> >> int bpf_strcat(char *dst, u32 dst__sz, const char *src);
> >> int bpf_strncat(char *dst, u32 dst__sz, const char *src, 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.
> >>
> >> The src parameter cannot directly pass a string constant, such as "XYZ",
> >> otherwise it will throw an error "write into map forbidden".
> >>
> >> Signed-off-by: Rong Tao <[email protected]>
> >> ---
> >> kernel/bpf/helpers.c | 75 ++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 75 insertions(+)
> >>
> >> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> >> index c18f1e16edee..85afff60365c 100644
> >> --- a/kernel/bpf/helpers.c
> >> +++ b/kernel/bpf/helpers.c
> >> @@ -4195,6 +4195,79 @@ __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 = 0, slen, space, copied;
> >> +
> >> + if (sz == 0 || dsz == 0)
> >> + return -EINVAL;
> >> +
> >> + while (dlen < dsz && dst[dlen] != '\0')
> >> + dlen++;
> >> +
> >> + if (dlen >= dsz)
> >> + return -E2BIG;
> >> +
> >> + space = dsz - dlen;
> >> +
> >> + slen = bpf_strnlen(src, sz);
> >> + if (space <= slen)
> >> + return -E2BIG;
> >> +
> >> + copied = strncpy_from_kernel_nofault(dst + dlen, src, sz);
> > I'm not sure strncpy_from_kernel_nofault() is designed to work
> > correctly if dst can be modified concurrently. Which in BPF case, even
> > though we validate that memory is correct, it still can be modified
> > concurrently in such a way as to cause strncpy_from_kernel_nofault's
> > inner dst[-1] based loop logic to go beyond the boundaries of target
> > buffer.
> >
> > i.e., I don't believe current implementation is concurrency safe
>
> Thank you very much for your reply. There is indeed a thread safety
> issue. Let's look at a solution. I will submit v6. Thank you ;)
>
tbh, I'm not sure we need these new kfuncs at all. You should be able
to easily to do the same with dynptr for dst buffer and existing
strlen/probe_read helpers/kfuncs.
> Rong Tao
>
> >
> > pw-bot: cr
> >
> >
> >> + if (copied < 0)
> >> + return copied;
> >> + else if (copied == 0 || copied == 1)
> >> + return dlen;
> >> +
> >> + /* The copied character count includes '\0'. */
> >> + return dlen + copied - 1;
> >> +}
> >> +
> >> +/**
> >> + * bpf_strcat - Append non-null bytes from a source string, and
> >> null-terminate
> >> + * the result
> >> + * @dst: Destination string.
> >> + * @dst__sz: Maximum bytes of @dst__ign, includes the trailing NUL.
> >> + * @src: 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, u32 dst__sz, const char *src)
> >> +{
> >> + return __bpf_strncat(dst, dst__sz, src, dst__sz);
> >> +}
> >> +
> >> +/**
> >> + * bpf_strncat - Append non-null bytes from a source string, and
> >> null-terminate
> >> + * the result
> >> + * @dst: Destination string.
> >> + * @dst__sz: Maximum bytes of @dst__ign, includes the trailing NUL.
> >> + * @src: Source string.
> >> + * @len: the maximum number of characters to concatenate, the null
> >> terminator
> >> + * \0 of the target string @dst will also be included.
> >> + *
> >> + * 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, u32 dst__sz, const char *src, u32
> >> len)
> >> +{
> >> + return __bpf_strncat(dst, dst__sz, src, len);
> >> +}
> >> +
> >> #ifdef CONFIG_KEYS
> >> /**
> >> * bpf_lookup_user_key - lookup a key by its serial
> >> @@ -4958,6 +5031,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
> >>
>
