Messages in this thread | | | Date | Fri, 5 Nov 2021 12:51:55 -0700 | From | Cong Wang <> | Subject | Re: [PATCH bpf-next v5 1/2] bpf: support BPF_PROG_QUERY for progs attached to sockmap |
| |
On Thu, Nov 04, 2021 at 09:07:44AM +0800, Di Zhu wrote: > +int sock_map_bpf_prog_query(const union bpf_attr *attr, > + union bpf_attr __user *uattr) > +{ > + __u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids); > + u32 prog_cnt = 0, flags = 0, ufd = attr->target_fd; > + struct bpf_prog **pprog; > + struct bpf_prog *prog; > + struct bpf_map *map; > + struct fd f; > + u32 id = 0; > + int ret; > + > + if (attr->query.query_flags) > + return -EINVAL; > + > + f = fdget(ufd); > + map = __bpf_map_get(f); > + if (IS_ERR(map)) > + return PTR_ERR(map); > + > + rcu_read_lock(); > + > + ret = sock_map_prog_lookup(map, &pprog, attr->query.attach_type); > + if (ret) > + goto end; > + > + prog = *pprog; > + prog_cnt = (!prog) ? 0 : 1; > + > + if (!attr->query.prog_cnt || !prog_ids || !prog_cnt) > + goto end;
This sanity check (except prog_cnt) can be moved before RCU read lock?
> + > + id = prog->aux->id; > + if (id == 0) > + prog_cnt = 0;
The id seems generic, so why not handle it in bpf_prog_query() for all progs?
> + > +end: > + rcu_read_unlock(); > + > + if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags)) ||
'flags' is always 0 here, right? So this is not needed as uattr has been already cleared in __sys_bpf().
Thanks.
| |