On Tue, Dec 3, 2024 at 5:48 AM Anton Protopopov <aspsk@xxxxxxxxxxxxx> wrote:
>
> The fd_array attribute of the BPF_PROG_LOAD syscall may contain a set
> of file descriptors: maps or btfs. This field was introduced as a
> sparse array. Introduce a new attribute, fd_array_cnt, which, if
> present, indicates that the fd_array is a continuous array of the
> corresponding length.
>
> If fd_array_cnt is non-zero, then every map in the fd_array will be
> bound to the program, as if it was used by the program. This
> functionality is similar to the BPF_PROG_BIND_MAP syscall, but such
> maps can be used by the verifier during the program load.
>
> Signed-off-by: Anton Protopopov <aspsk@xxxxxxxxxxxxx>
> ---
> include/uapi/linux/bpf.h | 10 ++++
> kernel/bpf/syscall.c | 2 +-
> kernel/bpf/verifier.c | 98 ++++++++++++++++++++++++++++------
> tools/include/uapi/linux/bpf.h | 10 ++++
> 4 files changed, 104 insertions(+), 16 deletions(-)
>
[...]
> +/*
> + * The add_fd_from_fd_array() is executed only if fd_array_cnt is non-zero. In
> + * this case expect that every file descriptor in the array is either a map or
> + * a BTF. Everything else is considered to be trash.
> + */
> +static int add_fd_from_fd_array(struct bpf_verifier_env *env, int fd)
> +{
> + struct bpf_map *map;
> + CLASS(fd, f)(fd);
> + int ret;
> +
> + map = __bpf_map_get(f);
> + if (!IS_ERR(map)) {
> + ret = __add_used_map(env, map);
> + if (ret < 0)
> + return ret;
> + return 0;
> + }
> +
> + /*
> + * Unlike "unused" maps which do not appear in the BPF program,
> + * BTFs are visible, so no reason to refcnt them now
What does "BTFs are visible" mean? I find this behavior surprising,
tbh. Map is added to used_maps, but BTF is *not* added to used_btfs?
Why?
> + */
> + if (!IS_ERR(__btf_get_by_fd(f)))
> + return 0;
> +
> + verbose(env, "fd %d is not pointing to valid bpf_map or btf\n", fd);
> + return PTR_ERR(map);
> +}
> +
> +static int process_fd_array(struct bpf_verifier_env *env, union bpf_attr *attr, bpfptr_t uattr)
> +{
> + size_t size = sizeof(int);
> + int ret;
> + int fd;
> + u32 i;
> +
> + env->fd_array = make_bpfptr(attr->fd_array, uattr.is_kernel);
> +
> + /*
> + * The only difference between old (no fd_array_cnt is given) and new
> + * APIs is that in the latter case the fd_array is expected to be
> + * continuous and is scanned for map fds right away
> + */
> + if (!attr->fd_array_cnt)
> + return 0;
> +
> + for (i = 0; i < attr->fd_array_cnt; i++) {
> + if (copy_from_bpfptr_offset(&fd, env->fd_array, i * size, size))
potential overflow in `i * size`? Do we limit fd_array_cnt anywhere to
less than INT_MAX/4?
> + return -EFAULT;
> +
> + ret = add_fd_from_fd_array(env, fd);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
[...]
