linux-kernel - Re: [PATCH bpf-next v7 4/5] bpf: Make fs kfuncs available for SYSCALL program type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <AM6PR03MB5080CDA2F6336B1BA2FDF2C199E12@AM6PR03MB5080.eurprd03.prod.outlook.com>
Date: Wed, 22 Jan 2025 13:34:08 +0000
From: Juntong Deng <juntong.deng@...look.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>,
 John Fastabend <john.fastabend@...il.com>,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...ux.dev>, Eddy Z <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
 KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
 Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
 Kumar Kartikeya Dwivedi <memxor@...il.com>, snorcht@...il.com,
 Christian Brauner <brauner@...nel.org>, bpf <bpf@...r.kernel.org>,
 LKML <linux-kernel@...r.kernel.org>,
 Linux-Fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH bpf-next v7 4/5] bpf: Make fs kfuncs available for SYSCALL
 program type

On 2025/1/22 00:43, Alexei Starovoitov wrote:
> On Tue, Jan 21, 2025 at 5:09 AM Juntong Deng <juntong.deng@...look.com> 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@...look.com>
>> ---
>>   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?

But if you think we should keep it, I will keep it in the next version.