On Tue, May 6, 2025 at 11:41 PM Viktor Malik <vmalik@xxxxxxxxxx> wrote:
>
> String operations are commonly used so this exposes the most common ones
> to BPF programs. For now, we limit ourselves to operations which do not
> copy memory around.
>
> Unfortunately, most in-kernel implementations assume that strings are
> %NUL-terminated, which is not necessarily true, and therefore we cannot
> use them directly in the BPF context. Instead, we open-code them using
> __get_kernel_nofault instead of plain dereference to make them safe and
> limit the strings length to XATTR_SIZE_MAX to make sure the functions
> terminate. When __get_kernel_nofault fails, functions return -EFAULT.
> Similarly, when the size bound is reached, the functions return -E2BIG.
>
> At the moment, strings can be passed to the kfuncs in three forms:
> - string literals (i.e. pointers to read-only maps)
> - global variables (i.e. pointers to read-write maps)
> - stack-allocated buffers
>
> Note that currently, it is not possible to pass strings from the BPF
> program context (like function args) as the verifier doesn't treat them
> as neither PTR_TO_MEM nor PTR_TO_BTF_ID.
>
> Suggested-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> Signed-off-by: Viktor Malik <vmalik@xxxxxxxxxx>
> ---
> kernel/bpf/helpers.c | 440 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 440 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index e3a2662f4e33..8fb7c2ca7ac0 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -23,6 +23,7 @@
> #include <linux/btf_ids.h>
> #include <linux/bpf_mem_alloc.h>
> #include <linux/kasan.h>
> +#include <linux/uaccess.h>
>
> #include "../../lib/kstrtox.h"
>
> @@ -3194,6 +3195,433 @@ __bpf_kfunc void bpf_local_irq_restore(unsigned long *flags__irq_flag)
> local_irq_restore(*flags__irq_flag);
> }
>
> +/* Kfuncs for string operations.
> + *
> + * Since strings are not necessarily %NUL-terminated, we cannot directly call
> + * in-kernel implementations. Instead, we open-code the implementations using
> + * __get_kernel_nofault instead of plain dereference to make them safe.
> + */
> +
> +/**
> + * bpf_strcmp - Compare two strings
> + * @s1: One string
> + * @s2: Another string
> + *
> + * Return:
> + * * %0 - Strings are equal
> + * * %-1 - @s1 is smaller
> + * * %1 - @s2 is smaller
> + * * %-EFAULT - Cannot read one of the strings
> + * * %-E2BIG - One of strings is too large
> + */
> +__bpf_kfunc int bpf_strcmp(const char *s1, const char *s2)
> +{
> + char c1, c2;
> + int i;
> +
> + if (!s1 || !s2)
> + return -EFAULT;
> +
> + guard(pagefault)();
> + for (i = 0; i < XATTR_SIZE_MAX; i++) {
> + __get_kernel_nofault(&c1, s1, char, err_out);
> + __get_kernel_nofault(&c2, s2, char, err_out);
> + if (c1 != c2)
> + return c1 < c2 ? -1 : 1;
> + if (c1 == '\0')
> + return 0;
> + s1++;
> + s2++;
> + }
> + return -E2BIG;
> +err_out:
> + return -EFAULT;
> +}
> +
> +/**
> + * bpf_strchr - Find the first occurrence of a character in a string
> + * @s: The string to be searched
> + * @c: The character to search for
> + *
> + * Note that the %NUL-terminator is considered part of the string, and can
> + * be searched for.
> + *
> + * Return:
> + * * const char * - Pointer to the first occurrence of @c within @s
> + * * %NULL - @c not found in @s
> + * * %-EFAULT - Cannot read @s
> + * * %-E2BIG - @s too large
> + */
> +__bpf_kfunc const char *bpf_strchr(const char *s, char c)
so let's say we found the character, we return a pointer to it, and
that memory goes away (because we never owned it, so we don't really
know what and when will happen with it). Question, will verifier allow
BPF program to dereference this pointer? If yes, that's a problem. But
if not, then I'm not sure there is much point in returning a pointer.
I'm just trying to imply that in BPF world integer-based APIs work
better/safer, overall? For strings, we can switch any
pointer-returning API to position-returning (or negative error) API
and it would more or less naturally fit into BPF API surface, no?
> +{
> + char sc;
> + int i;
> +
> + if (!s)
> + return ERR_PTR(-EFAULT);
> +
> + guard(pagefault)();
> + for (i = 0; i < XATTR_SIZE_MAX; i++) {
> + __get_kernel_nofault(&sc, s, char, err_out);
> + if (sc == c)
> + return s;
> + if (sc == '\0')
> + return NULL;
> + s++;
> + }
> + return ERR_PTR(-E2BIG);
> +err_out:
> + return ERR_PTR(-EFAULT);
this implementation can be replaced with just `return bpf_strnchr(s,
XATTR_SIZE_MAX, c);`, no?
> +}
> +
> +/**
> + * bpf_strnchr - Find a character in a length limited string
> + * @s: The string to be searched
> + * @count: The number of characters to be searched
> + * @c: The character to search for
> + *
> + * Note that the %NUL-terminator is considered part of the string, and can
> + * be searched for.
> + *
> + * Return:
> + * * const char * - Pointer to the first occurrence of @c within @s
> + * * %NULL - @c not found in the first @count characters of @s
> + * * %-EFAULT - Cannot read @s
> + * * %-E2BIG - @s too large
> + */
> +__bpf_kfunc const char *bpf_strnchr(const char *s, size_t count, char c)
> +{
> + char sc;
> + int i;
> +
> + if (!s)
> + return ERR_PTR(-EFAULT);
> +
> + guard(pagefault)();
> + for (i = 0; i < count && i < XATTR_SIZE_MAX; i++) {
> + __get_kernel_nofault(&sc, s, char, err_out);
> + if (sc == c)
> + return s;
> + if (sc == '\0')
> + return NULL;
> + s++;
> + }
> + return i == XATTR_SIZE_MAX ? ERR_PTR(-E2BIG) : NULL;
> +err_out:
> + return ERR_PTR(-EFAULT);
> +}
> +
[...]
> +/**
> + * bpf_strlen - Calculate the length of a string
> + * @s: The string
> + *
> + * Return:
> + * * >=0 - The length of @s
> + * * %-EFAULT - Cannot read @s
> + * * %-E2BIG - @s too large
> + */
> +__bpf_kfunc int bpf_strlen(const char *s)
> +{
> + char c;
> + int i;
> +
> + if (!s)
> + return -EFAULT;
> +
> + guard(pagefault)();
> + for (i = 0; i < XATTR_SIZE_MAX; i++) {
> + __get_kernel_nofault(&c, s, char, err_out);
> + if (c == '\0')
> + return i;
> + s++;
> + }
> + return -E2BIG;
> +err_out:
> + return -EFAULT;
return bpf_strnlen(s, XATTR_SIZE_MAX)?
You get the idea.
[...]
