On Tue, Oct 17, 2023 at 9:11 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Fri, Oct 13, 2023 at 11:30 AM Song Liu <song@xxxxxxxxxx> wrote:
> > +__bpf_kfunc int bpf_get_file_xattr(struct file *file, struct bpf_dynptr_kern *name_ptr,
> > + struct bpf_dynptr_kern *value_ptr)
> > +{
> > + if (!bpf_dynptr_is_string(name_ptr))
> > + return -EINVAL;
> > +
> > + return vfs_getxattr(mnt_idmap(file->f_path.mnt), file_dentry(file), name_ptr->data,
> > + value_ptr->data, __bpf_dynptr_size(value_ptr));
> > +}
> > +
> > +__diag_pop();
> > +
> > +BTF_SET8_START(fs_kfunc_set)
> > +BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE)
>
> I suspect it needs to be allowlisted too.
> Sleepable might not be enough.
>
> KP proposed such kfunc in the past and there were recursion issues.
>
> KP,
> do you remember the details?
yeah, have a look at Al's reply:
https://lore.kernel.org/bpf/Yrs4+ThR4ACb5eD%2F@ZenIV/
it can create deadlocks and potentially UAFs (similar to the situation
Jann mentioned). This will need to be allowlisted.
