On Thu, Jul 06, 2023 at 09:22:29PM -0700, Andrii Nakryiko wrote:
SNIP
> > + flags = attr->link_create.uprobe_multi.flags;
> > + if (flags & ~BPF_F_UPROBE_MULTI_RETURN)
> > + return -EINVAL;
> > +
> > + /*
> > + * path, offsets and cnt are mandatory,
> > + * ref_ctr_offsets is optional
> > + */
> > + upath = u64_to_user_ptr(attr->link_create.uprobe_multi.path);
> > + uoffsets = u64_to_user_ptr(attr->link_create.uprobe_multi.offsets);
> > + cnt = attr->link_create.uprobe_multi.cnt;
> > +
> > + if (!upath || !uoffsets || !cnt)
> > + return -EINVAL;
>
> see below for -EBADF, but we can also, additionally, return -EPROTO
> here, for example?
>
> > +
> > + uref_ctr_offsets = u64_to_user_ptr(attr->link_create.uprobe_multi.ref_ctr_offsets);
> > +
> > + name = strndup_user(upath, PATH_MAX);
> > + if (IS_ERR(name)) {
> > + err = PTR_ERR(name);
> > + return err;
> > + }
> > +
> > + err = kern_path(name, LOOKUP_FOLLOW, &path);
> > + kfree(name);
> > + if (err)
> > + return err;
> > +
> > + if (!d_is_reg(path.dentry)) {
> > + err = -EINVAL;
>
> as I mentioned in another patch, -EBADF here for feature detection
> (and it makes sense by itself, probably)
yes, I like this place better, also -EBADF error makes more sense in here
thanks,
jirka
>
> > + goto error_path_put;
> > + }
> > +
> > + err = -ENOMEM;
> > +
> > + link = kzalloc(sizeof(*link), GFP_KERNEL);
> > + uprobes = kvcalloc(cnt, sizeof(*uprobes), GFP_KERNEL);
> > +
> > + if (!uprobes || !link)
> > + goto error_free;
> > +
> > + if (uref_ctr_offsets) {
> > + ref_ctr_offsets = kvcalloc(cnt, sizeof(*ref_ctr_offsets), GFP_KERNEL);
> > + if (!ref_ctr_offsets)
> > + goto error_free;
> > + }
> > +
>
> [...]
