On Fri, Jan 24, 2025 at 10:33 AM Jordan Rome <linux@xxxxxxxxxxxxxx> wrote:
>
> This new kfunc will be able to copy a string
> from another process's/task's address space.
> This is similar to `bpf_copy_from_user_str`
> but accepts a `struct task_struct*` argument.
>
> Signed-off-by: Jordan Rome <linux@xxxxxxxxxxxxxx>
> ---
> kernel/bpf/helpers.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index f27ce162427a..c26fabf97afd 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -3082,6 +3082,56 @@ __bpf_kfunc void bpf_local_irq_restore(unsigned long *flags__irq_flag)
> local_irq_restore(*flags__irq_flag);
> }
>
> +/**
> + * bpf_copy_from_user_task_str() - Copy a string from an task's address space
> + * @dst: Destination address, in kernel space. This buffer must be
> + * at least @dst__sz bytes long.
> + * @dst__sz: Maximum number of bytes to copy, includes the trailing NUL.
> + * @unsafe_ptr__ign: Source address in the task's address space.
> + * @tsk: The task whose address space will be used
> + * @flags: The only supported flag is BPF_F_PAD_ZEROS
> + *
> + * Copies a NULL-terminated string from a task's address space to *dst* buffer.
> + * If user string is too long this will still ensure zero termination in the
> + * dst buffer unless buffer size is 0.
> + *
> + * If BPF_F_PAD_ZEROS flag is set, memset the tail of @dst to 0 on success and
> + * memset all of @dst on failure.
> + *
> + * Return: The number of copied bytes on success, including the NULL-terminator.
s/NULL/NUL/
Other than that, LGTM:
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> + * A negative error code on failure.
> + */
> +__bpf_kfunc int bpf_copy_from_user_task_str(void *dst,
> + u32 dst__sz,
> + const void __user *unsafe_ptr__ign,
> + struct task_struct *tsk,
> + u64 flags)
> +{
> + int ret = 0;
nit: no need to zero initialize, you'll overwrite it unconditionally below
> +
> + if (unlikely(flags & ~BPF_F_PAD_ZEROS))
> + return -EINVAL;
> +
> + if (unlikely(!dst__sz))
> + return 0;
> +
> + ret = copy_remote_vm_str(tsk, (unsigned long)unsafe_ptr__ign, dst, dst__sz, 0);
> +
> + if (ret <= 0) {
> + if (flags & BPF_F_PAD_ZEROS)
> + memset(dst, 0, dst__sz);
> + return ret ?: -EINVAL;
> + }
> +
> + if (ret < dst__sz) {
> + if (flags & BPF_F_PAD_ZEROS)
> + memset(dst + ret, 0, dst__sz - ret);
> + return ret + 1;
> + }
> +
> + return ret;
> +}
> +
> __bpf_kfunc_end_defs();
>
> BTF_KFUNCS_START(generic_btf_ids)
> @@ -3174,6 +3224,7 @@ 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_copy_from_user_task_str, KF_SLEEPABLE)
> BTF_ID_FLAGS(func, bpf_get_kmem_cache)
> BTF_ID_FLAGS(func, bpf_iter_kmem_cache_new, KF_ITER_NEW | KF_SLEEPABLE)
> BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLEEPABLE)
> --
> 2.43.5
>
