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

[Andrii Nakryiko]

On Tue, Jul 21, 2026 at 9:42 PM Rong Tao <rtoax@xxxxxxxxxxx> wrote:
>
> From: Rong Tao <rongtao@xxxxxxxx>
>
> 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 <rongtao@xxxxxxxx>
> ---
>  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

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
>