On Wed, Sep 25, 2024 at 11:18 PM Viktor Malik <vmalik@xxxxxxxxxx> wrote:
>
> Kernel contains highly optimised implementation of traditional string
> operations. Expose them as kfuncs to allow BPF programs leverage the
> kernel implementation instead of needing to reimplement the operations.
>
> For now, add kfuncs for all string operations which do not copy memory
> around: strcmp, strchr, strrchr, strnchr, strstr, strnstr, strlen,
> strnlen, strpbrk, strspn, strcspn. Do not add strncmp as it is already
> exposed as a helper.
>
> Signed-off-by: Viktor Malik <vmalik@xxxxxxxxxx>
> ---
> kernel/bpf/helpers.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 1a43d06eab28..daa19760d8c8 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -3004,6 +3004,61 @@ __bpf_kfunc int bpf_copy_from_user_str(void *dst, u32 dst__sz, const void __user
> return ret + 1;
> }
>
> +__bpf_kfunc int bpf_strcmp(const char *cs, const char *ct)
I don't think this will work as you hope it will work. I suspect BPF
verifier thinks you are asking to a pointer to a singular char (1-byte
memory, basically), and expects kfunc to not access beyond that single
byte. Which is not what you are doing, so it's a violation of declared
kfunc contract.
We do have support to pass constant string pointers that are known at
verification time (see bpf_strncmp() and also is_kfunc_arg_const_str()
for kfunc-like equivalent), but I don't think that's what you want
here.
Right now, the only way to pass dynamically sized anything is through
dynptr, AFAIU.
pw-bot: cr
> +{
> + return strcmp(cs, ct);
> +}
> +
> +__bpf_kfunc char *bpf_strchr(const char *s, int c)
> +{
> + return strchr(s, c);
> +}
> +
> +__bpf_kfunc char *bpf_strrchr(const char *s, int c)
> +{
> + return strrchr(s, c);
> +}
> +
> +__bpf_kfunc char *bpf_strnchr(const char *s, size_t count, int c)
> +{
> + return strnchr(s, count, c);
> +}
> +
> +__bpf_kfunc char *bpf_strstr(const char *s1, const char *s2)
> +{
> + return strstr(s1, s2);
> +}
> +
> +__bpf_kfunc char *bpf_strnstr(const char *s1, const char *s2, size_t len)
> +{
> + return strnstr(s1, s2, len);
> +}
> +
> +__bpf_kfunc size_t bpf_strlen(const char *s)
> +{
> + return strlen(s);
> +}
> +
> +__bpf_kfunc size_t bpf_strnlen(const char *s, size_t count)
> +{
> + return strnlen(s, count);
> +}
> +
> +__bpf_kfunc char *bpf_strpbrk(const char *cs, const char *ct)
> +{
> + return strpbrk(cs, ct);
> +}
> +
> +__bpf_kfunc size_t bpf_strspn(const char *s, const char *accept)
> +{
> + return strspn(s, accept);
> +}
> +
> +__bpf_kfunc size_t bpf_strcspn(const char *s, const char *reject)
> +{
> + return strcspn(s, reject);
> +}
> +
> __bpf_kfunc_end_defs();
>
> BTF_KFUNCS_START(generic_btf_ids)
> @@ -3090,6 +3145,17 @@ BTF_ID_FLAGS(func, bpf_iter_bits_new, KF_ITER_NEW)
> BTF_ID_FLAGS(func, bpf_iter_bits_next, KF_ITER_NEXT | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_iter_bits_destroy, KF_ITER_DESTROY)
> BTF_ID_FLAGS(func, bpf_copy_from_user_str, KF_SLEEPABLE)
> +BTF_ID_FLAGS(func, bpf_strcmp)
> +BTF_ID_FLAGS(func, bpf_strchr)
> +BTF_ID_FLAGS(func, bpf_strrchr)
> +BTF_ID_FLAGS(func, bpf_strnchr)
> +BTF_ID_FLAGS(func, bpf_strstr)
> +BTF_ID_FLAGS(func, bpf_strnstr)
> +BTF_ID_FLAGS(func, bpf_strlen)
> +BTF_ID_FLAGS(func, bpf_strnlen)
> +BTF_ID_FLAGS(func, bpf_strpbrk)
> +BTF_ID_FLAGS(func, bpf_strspn)
> +BTF_ID_FLAGS(func, bpf_strcspn)
> BTF_KFUNCS_END(common_btf_ids)
>
> static const struct btf_kfunc_id_set common_kfunc_set = {
> --
> 2.46.0
>
