On Wed, Jan 22, 2025 at 5:34 AM Juntong Deng <juntong.deng@xxxxxxxxxxx> wrote:
>
> On 2025/1/22 00:43, Alexei Starovoitov wrote:
> > On Tue, Jan 21, 2025 at 5:09 AM Juntong Deng <juntong.deng@xxxxxxxxxxx> wrote:
> >>
> >> Currently fs kfuncs are only available for LSM program type, but fs
> >> kfuncs are generic and useful for scenarios other than LSM.
> >>
> >> This patch makes fs kfuncs available for SYSCALL program type.
> >>
> >> Signed-off-by: Juntong Deng <juntong.deng@xxxxxxxxxxx>
> >> ---
> >> fs/bpf_fs_kfuncs.c | 14 ++++++--------
> >> .../selftests/bpf/progs/verifier_vfs_reject.c | 10 ----------
> >> 2 files changed, 6 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> >> index 4a810046dcf3..8a7e9ed371de 100644
> >> --- a/fs/bpf_fs_kfuncs.c
> >> +++ b/fs/bpf_fs_kfuncs.c
> >> @@ -26,8 +26,6 @@ __bpf_kfunc_start_defs();
> >> * acquired by this BPF kfunc will result in the BPF program being rejected by
> >> * the BPF verifier.
> >> *
> >> - * This BPF kfunc may only be called from BPF LSM programs.
> >> - *
> >> * Internally, this BPF kfunc leans on get_task_exe_file(), such that calling
> >> * bpf_get_task_exe_file() would be analogous to calling get_task_exe_file()
> >> * directly in kernel context.
> >> @@ -49,8 +47,6 @@ __bpf_kfunc struct file *bpf_get_task_exe_file(struct task_struct *task)
> >> * passed to this BPF kfunc. Attempting to pass an unreferenced file pointer, or
> >> * any other arbitrary pointer for that matter, will result in the BPF program
> >> * being rejected by the BPF verifier.
> >> - *
> >> - * This BPF kfunc may only be called from BPF LSM programs.
> >> */
> >> __bpf_kfunc void bpf_put_file(struct file *file)
> >> {
> >> @@ -70,8 +66,6 @@ __bpf_kfunc void bpf_put_file(struct file *file)
> >> * reference, or else the BPF program will be outright rejected by the BPF
> >> * verifier.
> >> *
> >> - * This BPF kfunc may only be called from BPF LSM programs.
> >> - *
> >> * Return: A positive integer corresponding to the length of the resolved
> >> * pathname in *buf*, including the NUL termination character. On error, a
> >> * negative integer is returned.
> >> @@ -184,7 +178,8 @@ BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)
> >> static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id)
> >> {
> >> if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) ||
> >> - prog->type == BPF_PROG_TYPE_LSM)
> >> + prog->type == BPF_PROG_TYPE_LSM ||
> >> + prog->type == BPF_PROG_TYPE_SYSCALL)
> >> return 0;
> >> return -EACCES;
> >> }
> >> @@ -197,7 +192,10 @@ static const struct btf_kfunc_id_set bpf_fs_kfunc_set = {
> >>
> >> static int __init bpf_fs_kfuncs_init(void)
> >> {
> >> - return register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM, &bpf_fs_kfunc_set);
> >> + int ret;
> >> +
> >> + ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM, &bpf_fs_kfunc_set);
> >> + return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_fs_kfunc_set);
> >> }
> >>
> >> late_initcall(bpf_fs_kfuncs_init);
> >> diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
> >> index d6d3f4fcb24c..5aab75fd2fa5 100644
> >> --- a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
> >> +++ b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
> >> @@ -148,14 +148,4 @@ int BPF_PROG(path_d_path_kfunc_invalid_buf_sz, struct file *file)
> >> return 0;
> >> }
> >>
> >> -SEC("fentry/vfs_open")
> >> -__failure __msg("calling kernel function bpf_path_d_path is not allowed")
> >> -int BPF_PROG(path_d_path_kfunc_non_lsm, struct path *path, struct file *f)
> >> -{
> >> - /* Calling bpf_path_d_path() from a non-LSM BPF program isn't permitted.
> >> - */
> >> - bpf_path_d_path(path, buf, sizeof(buf));
> >> - return 0;
> >> -}
> >
> > A leftover from previous versions?
> > This test should still be rejected by the verifier.
>
> Thanks for your reply.
>
> Not a leftover.
>
> bpf_path_d_path can be called from SYSCALL program type, not only LSM
> program type, so it seems a bit weird to keep this test case?
How is it weird?
How is this related to syscall prog?
It's a check that fentry prog cannot call it.
