Re: [PATCH bpf-next 3/5] bpf: add fd_array_cnt attribute for prog_load

[Anton Protopopov <aspsk@isovalent.com>]

On 24/11/15 07:06PM, Eduard Zingerman wrote:
> On Fri, 2024-11-15 at 00:46 +0000, Anton Protopopov wrote:
> 
> [...]
> 
> > @@ -22537,6 +22543,76 @@ struct btf *bpf_get_btf_vmlinux(void)
> >  	return btf_vmlinux;
> >  }
> >  
> > +/*
> > + * The add_fd_from_fd_array() is executed only if fd_array_cnt is given.  In
> > + * this case expect that every file descriptor in the array is either a map or
> > + * a BTF, or a hole (0). 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)) {
> > +		if (!IS_ERR(__btf_get_by_fd(f)))
> > +			return 0;
> > +
> > +		/* allow holes */
> > +		if (!fd)
> > +			return 0;
> > +
> > +		verbose(env, "fd %d is not pointing to valid bpf_map or btf\n", fd);
> > +		return PTR_ERR(map);
> > +	}
> > +
> > +	ret = add_used_map(env, map);
> > +	if (ret < 0)
> > +		return ret;
> > +	return 0;
> > +}
> 
> Nit: keeping this function "flat" would allow easier extension, if necessary.
>      E.g.:
> 
>     static int add_fd_from_fd_array(struct bpf_verifier_env *env, int fd)
>     {
>     	struct bpf_map *map;
>     	CLASS(fd, f)(fd);
>     	int ret;
> 
>     	/* allow holes */
>     	if (!fd) {
>     		return 0;
>     	}
>     	map = __bpf_map_get(f);
>     	if (!IS_ERR(map)) {
>     		ret = add_used_map(env, map);
>     		return ret < 0 ? ret : 0;
>     	}
>     	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 -EINVAL;
>     }

Thanks, this makes sense, I will change it to a flat version in v2.

> [...]