[PATCH v4 bpf-next 4/6] bpf: fs/xattr: Add BPF kfuncs to set and remove xattrs

[Song Liu]

Hi Alexei, 

Thanks for the review!

> On Dec 17, 2024, at 8:50 AM, Alexei Starovoitov <alexei.starovoitov at gmail.com> wrote:
> 
> On Mon, Dec 16, 2024 at 10:38 PM Song Liu <song at kernel.org> wrote:
>> 
>> Add the following kfuncs to set and remove xattrs from BPF programs:
>> 
>>  bpf_set_dentry_xattr
>>  bpf_remove_dentry_xattr
>>  bpf_set_dentry_xattr_locked
>>  bpf_remove_dentry_xattr_locked
>> 
>> The _locked version of these kfuncs are called from hooks where
>> dentry->d_inode is already locked.
> 
> ...
> 
>> + *
>> + * Setting and removing xattr requires exclusive lock on dentry->d_inode.
>> + * Some hooks already locked d_inode, while some hooks have not locked
>> + * d_inode. Therefore, we need different kfuncs for different hooks.
>> + * Specifically, hooks in the following list (d_inode_locked_hooks)
>> + * should call bpf_[set|remove]_dentry_xattr_locked; while other hooks
>> + * should call bpf_[set|remove]_dentry_xattr.
>> + */
> 
> the inode locking rules might change, so let's hide this
> implementation detail from the bpf progs by making kfunc polymorphic.
> 
> To struct bpf_prog_aux add:
> bool use_locked_kfunc:1;
> and set it in bpf_check_attach_target() if it's attaching
> to one of d_inode_locked_hooks
> 
> Then in fixup_kfunc_call() call some helper that
> if (prog->aux->use_locked_kfunc &&
>    insn->imm == special_kfunc_list[KF_bpf_remove_dentry_xattr])
>     insn->imm = special_kfunc_list[KF_bpf_remove_dentry_xattr_locked];
> 
> The progs will be simpler and will suffer less churn
> when the kernel side changes.

I was thinking about something in similar direction. 

If we do this, shall we somehow hide the _locked version of the 
kfuncs, so that the user cannot use it? If so, what's the best 
way to do it? 

Thanks,
Song