| Message ID | AM6PR03MB50804FA149F08D34A095BA28993D2@AM6PR03MB5080.eurprd03.prod.outlook.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bpf: Add open-coded style process file iterator and bpf_fget_task() kfunc | expand |
On Tue, Dec 10, 2024 at 02:03:53PM +0000, Juntong Deng 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 and TRACING > program types. I would like a detailed explanation from the maintainers what it means to make this available to SYSCALL program types, please.
On Tue, Dec 10, 2024 at 6:43 AM Christian Brauner <brauner@kernel.org> wrote: > > On Tue, Dec 10, 2024 at 02:03:53PM +0000, Juntong Deng 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 and TRACING > > program types. > > I would like a detailed explanation from the maintainers what it means > to make this available to SYSCALL program types, please. Sigh. This is obviously not safe from tracing progs. From BPF_PROG_TYPE_SYSCALL these kfuncs should be safe to use, since those progs are not attached to anything. Such progs can only be executed via sys_bpf syscall prog_run command. They're sleepable, preemptable, faultable, in task ctx. But I'm not sure what's the value of enabling these kfuncs for BPF_PROG_TYPE_SYSCALL.
On 2024/12/10 18:58, Alexei Starovoitov wrote: > On Tue, Dec 10, 2024 at 6:43 AM Christian Brauner <brauner@kernel.org> wrote: >> >> On Tue, Dec 10, 2024 at 02:03:53PM +0000, Juntong Deng 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 and TRACING >>> program types. >> >> I would like a detailed explanation from the maintainers what it means >> to make this available to SYSCALL program types, please. > > Sigh. > This is obviously not safe from tracing progs. > > From BPF_PROG_TYPE_SYSCALL these kfuncs should be safe to use, > since those progs are not attached to anything. > Such progs can only be executed via sys_bpf syscall prog_run command. > They're sleepable, preemptable, faultable, in task ctx. > > But I'm not sure what's the value of enabling these kfuncs for > BPF_PROG_TYPE_SYSCALL. Thanks for your reply. Song said here that we need some of these kfuncs to be available for tracing functions [0]. If Song saw this email, could you please join the discussion? [0]: https://lore.kernel.org/bpf/CAPhsuW6ud21v2xz8iSXf=CiDL+R_zpQ+p8isSTMTw=EiJQtRSw@mail.gmail.com/ For BPF_PROG_TYPE_SYSCALL, I think BPF_PROG_TYPE_SYSCALL has now exceeded its original designed purpose and has become a more general program type. Currently BPF_PROG_TYPE_SYSCALL is widely used in HID drivers, and there are some use cases in sched-ext (CRIB is also a use case, although still in infancy). As BPF_PROG_TYPE_SYSCALL becomes more general, it would be valuable to make more kfuncs available for BPF_PROG_TYPE_SYSCALL.
On Wed, Dec 11, 2024 at 1:29 PM Juntong Deng <juntong.deng@outlook.com> wrote: > > On 2024/12/10 18:58, Alexei Starovoitov wrote: > > On Tue, Dec 10, 2024 at 6:43 AM Christian Brauner <brauner@kernel.org> wrote: > >> > >> On Tue, Dec 10, 2024 at 02:03:53PM +0000, Juntong Deng 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 and TRACING > >>> program types. > >> > >> I would like a detailed explanation from the maintainers what it means > >> to make this available to SYSCALL program types, please. > > > > Sigh. > > This is obviously not safe from tracing progs. > > > > From BPF_PROG_TYPE_SYSCALL these kfuncs should be safe to use, > > since those progs are not attached to anything. > > Such progs can only be executed via sys_bpf syscall prog_run command. > > They're sleepable, preemptable, faultable, in task ctx. > > > > But I'm not sure what's the value of enabling these kfuncs for > > BPF_PROG_TYPE_SYSCALL. > > Thanks for your reply. > > Song said here that we need some of these kfuncs to be available for > tracing functions [0]. I meant we can put the new kfuncs, such as bpf_get_file_ops_type, in bpf_fs_kfuncs.c, and make it available to tracing programs. But we cannot blindly make all of these kfuncs available to tracing programs. Instead, we need to review each kfunc and check whether it is safe for tracing programs. Thanks, Song > If Song saw this email, could you please join the discussion? > > [0]: > https://lore.kernel.org/bpf/CAPhsuW6ud21v2xz8iSXf=CiDL+R_zpQ+p8isSTMTw=EiJQtRSw@mail.gmail.com/
On Wed, Dec 11, 2024 at 1:29 PM Juntong Deng <juntong.deng@outlook.com> wrote: > > On 2024/12/10 18:58, Alexei Starovoitov wrote: > > On Tue, Dec 10, 2024 at 6:43 AM Christian Brauner <brauner@kernel.org> wrote: > >> > >> On Tue, Dec 10, 2024 at 02:03:53PM +0000, Juntong Deng 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 and TRACING > >>> program types. > >> > >> I would like a detailed explanation from the maintainers what it means > >> to make this available to SYSCALL program types, please. > > > > Sigh. > > This is obviously not safe from tracing progs. > > > > From BPF_PROG_TYPE_SYSCALL these kfuncs should be safe to use, > > since those progs are not attached to anything. > > Such progs can only be executed via sys_bpf syscall prog_run command. > > They're sleepable, preemptable, faultable, in task ctx. > > > > But I'm not sure what's the value of enabling these kfuncs for > > BPF_PROG_TYPE_SYSCALL. > > Thanks for your reply. > > Song said here that we need some of these kfuncs to be available for > tracing functions [0]. > > If Song saw this email, could you please join the discussion? > > [0]: > https://lore.kernel.org/bpf/CAPhsuW6ud21v2xz8iSXf=CiDL+R_zpQ+p8isSTMTw=EiJQtRSw@mail.gmail.com/ > > For BPF_PROG_TYPE_SYSCALL, I think BPF_PROG_TYPE_SYSCALL has now > exceeded its original designed purpose and has become a more general > program type. > > Currently BPF_PROG_TYPE_SYSCALL is widely used in HID drivers, and there > are some use cases in sched-ext (CRIB is also a use case, although still > in infancy). hid switched to use struct_ops prog type. I believe syscall prog type in hid is a legacy code. Those still present might be leftovers for older kernels. sched-ext is struct_ops only. No syscall progs there. > As BPF_PROG_TYPE_SYSCALL becomes more general, it would be valuable to > make more kfuncs available for BPF_PROG_TYPE_SYSCALL. Maybe. I still don't understand how it helps CRIB goal.
On 2024/12/11 22:06, Song Liu wrote: > On Wed, Dec 11, 2024 at 1:29 PM Juntong Deng <juntong.deng@outlook.com> wrote: >> >> On 2024/12/10 18:58, Alexei Starovoitov wrote: >>> On Tue, Dec 10, 2024 at 6:43 AM Christian Brauner <brauner@kernel.org> wrote: >>>> >>>> On Tue, Dec 10, 2024 at 02:03:53PM +0000, Juntong Deng 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 and TRACING >>>>> program types. >>>> >>>> I would like a detailed explanation from the maintainers what it means >>>> to make this available to SYSCALL program types, please. >>> >>> Sigh. >>> This is obviously not safe from tracing progs. >>> >>> From BPF_PROG_TYPE_SYSCALL these kfuncs should be safe to use, >>> since those progs are not attached to anything. >>> Such progs can only be executed via sys_bpf syscall prog_run command. >>> They're sleepable, preemptable, faultable, in task ctx. >>> >>> But I'm not sure what's the value of enabling these kfuncs for >>> BPF_PROG_TYPE_SYSCALL. >> >> Thanks for your reply. >> >> Song said here that we need some of these kfuncs to be available for >> tracing functions [0]. > > I meant we can put the new kfuncs, such as bpf_get_file_ops_type, in > bpf_fs_kfuncs.c, and make it available to tracing programs. But we > cannot blindly make all of these kfuncs available to tracing programs. > Instead, we need to review each kfunc and check whether it is safe > for tracing programs. > > Thanks, > Song > Thanks for joining the discussion. Yes, we should do that. Sorry for the misunderstanding. >> If Song saw this email, could you please join the discussion? >> >> [0]: >> https://lore.kernel.org/bpf/CAPhsuW6ud21v2xz8iSXf=CiDL+R_zpQ+p8isSTMTw=EiJQtRSw@mail.gmail.com/
On 2024/12/12 00:53, Alexei Starovoitov wrote: > On Wed, Dec 11, 2024 at 1:29 PM Juntong Deng <juntong.deng@outlook.com> wrote: >> >> On 2024/12/10 18:58, Alexei Starovoitov wrote: >>> On Tue, Dec 10, 2024 at 6:43 AM Christian Brauner <brauner@kernel.org> wrote: >>>> >>>> On Tue, Dec 10, 2024 at 02:03:53PM +0000, Juntong Deng 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 and TRACING >>>>> program types. >>>> >>>> I would like a detailed explanation from the maintainers what it means >>>> to make this available to SYSCALL program types, please. >>> >>> Sigh. >>> This is obviously not safe from tracing progs. >>> >>> From BPF_PROG_TYPE_SYSCALL these kfuncs should be safe to use, >>> since those progs are not attached to anything. >>> Such progs can only be executed via sys_bpf syscall prog_run command. >>> They're sleepable, preemptable, faultable, in task ctx. >>> >>> But I'm not sure what's the value of enabling these kfuncs for >>> BPF_PROG_TYPE_SYSCALL. >> >> Thanks for your reply. >> >> Song said here that we need some of these kfuncs to be available for >> tracing functions [0]. >> >> If Song saw this email, could you please join the discussion? >> >> [0]: >> https://lore.kernel.org/bpf/CAPhsuW6ud21v2xz8iSXf=CiDL+R_zpQ+p8isSTMTw=EiJQtRSw@mail.gmail.com/ >> >> For BPF_PROG_TYPE_SYSCALL, I think BPF_PROG_TYPE_SYSCALL has now >> exceeded its original designed purpose and has become a more general >> program type. >> >> Currently BPF_PROG_TYPE_SYSCALL is widely used in HID drivers, and there >> are some use cases in sched-ext (CRIB is also a use case, although still >> in infancy). > > hid switched to use struct_ops prog type. > I believe syscall prog type in hid is a legacy code. > Those still present might be leftovers for older kernels. > > sched-ext is struct_ops only. No syscall progs there. > I saw some on Github [0], sorry, yes they are not in the Linux tree. [0]: https://github.com/search?q=repo%3Asched-ext%2Fscx%20SEC(%22syscall%22)&type=code >> As BPF_PROG_TYPE_SYSCALL becomes more general, it would be valuable to >> make more kfuncs available for BPF_PROG_TYPE_SYSCALL. > > Maybe. I still don't understand how it helps CRIB goal. For CRIB goals, the program type is not important. What is important is that CRIB bpf programs are able to call the required kfuncs, and that CRIB ebpf programs can be executed from userspace. In our previous discussion, the conclusion was that we do not need a separate CRIB program type [1]. BPF_PROG_TYPE_SYSCALL can be executed from userspace via prog_run, which fits the CRIB use case of calling the ebpf program from userspace to get process information. So BPF_PROG_TYPE_SYSCALL becomes an option. [1]: https://lore.kernel.org/bpf/etzm4h5qm2jhgi6d4pevooy2sebrvgb3lsa67ym4x7zbh5bgnj@feoli4hj22so/ In fs/bpf_fs_kfuncs.c, CRIB currently needs bpf_fget_task (dump files opened by the process), bpf_put_file, and bpf_get_task_exe_file. So I would like these kfuncs to be available for BPF_PROG_TYPE_SYSCALL. bpf_get_dentry_xattr, bpf_get_file_xattr, and bpf_path_d_path have nothing to do with CRIB, but they are all in bpf_fs_kfunc_set_ids. Should we make bpf_fs_kfunc_set_ids available to BPF_PROG_TYPE_SYSCALL as a whole? Or create a separate set? Maybe we can discuss.
On Fri, Dec 13, 2024 at 10:51 AM Juntong Deng <juntong.deng@outlook.com> wrote: > > > > > sched-ext is struct_ops only. No syscall progs there. > > > > I saw some on Github [0], sorry, yes they are not in the Linux tree. > > [0]: > https://github.com/search?q=repo%3Asched-ext%2Fscx%20SEC(%22syscall%22)&type=code Ahh. I see. Those are executed from user space via prog_run. https://github.com/sched-ext/scx/blob/e8e68e8ee80f65f62a6e900d457306217b764e58/scheds/rust/scx_lavd/src/main.rs#L794 These progs are not executed by sched-ext core, so not really sched-ext progs. They're auxiliary progs that populate configs and knobs in bpf maps that sched-ext progs use later. > > >> As BPF_PROG_TYPE_SYSCALL becomes more general, it would be valuable to > >> make more kfuncs available for BPF_PROG_TYPE_SYSCALL. > > > > Maybe. I still don't understand how it helps CRIB goal. > > For CRIB goals, the program type is not important. What is important is > that CRIB bpf programs are able to call the required kfuncs, and that > CRIB ebpf programs can be executed from userspace. > > In our previous discussion, the conclusion was that we do not need a > separate CRIB program type [1]. > > BPF_PROG_TYPE_SYSCALL can be executed from userspace via prog_run, which > fits the CRIB use case of calling the ebpf program from userspace to get > process information. > > So BPF_PROG_TYPE_SYSCALL becomes an option. > > [1]: > https://lore.kernel.org/bpf/etzm4h5qm2jhgi6d4pevooy2sebrvgb3lsa67ym4x7zbh5bgnj@feoli4hj22so/ > > In fs/bpf_fs_kfuncs.c, CRIB currently needs bpf_fget_task (dump files > opened by the process), bpf_put_file, and bpf_get_task_exe_file. > > So I would like these kfuncs to be available for BPF_PROG_TYPE_SYSCALL. > > bpf_get_dentry_xattr, bpf_get_file_xattr, and bpf_path_d_path have > nothing to do with CRIB, but they are all in bpf_fs_kfunc_set_ids. > > Should we make bpf_fs_kfunc_set_ids available to BPF_PROG_TYPE_SYSCALL > as a whole? Or create a separate set? Maybe we can discuss. I don't think it's necessary to slide and dice that match. Since they're all safe from syscall prog it's cleaner to enable them all. When I said: > I still don't understand how it helps CRIB goal. I meant how are you going to use them from CRIB ? Patch 5 selftest does: + file = bpf_fget_task(task, test_fd1); + if (file == NULL) { + err = 2; + return 0; + } + + if (file->f_op != &pipefifo_fops) { + err = 3; + bpf_put_file(file); + return 0; + } + + bpf_put_file(file); It's ok for selftest, but not enough to explain the motivation and end-to-end operation of CRIB. Patch 2 selftest is also weak. It's not using bpf_iter_task_file_next() like iterators are normally used. When selftests are basic sanity tests, it begs the question: what's next? How are they going to be used for real?
On 2024/12/14 00:41, Alexei Starovoitov wrote: > On Fri, Dec 13, 2024 at 10:51 AM Juntong Deng <juntong.deng@outlook.com> wrote: >> >>> >>> sched-ext is struct_ops only. No syscall progs there. >>> >> >> I saw some on Github [0], sorry, yes they are not in the Linux tree. >> >> [0]: >> https://github.com/search?q=repo%3Asched-ext%2Fscx%20SEC(%22syscall%22)&type=code > > Ahh. I see. Those are executed from user space via prog_run. > https://github.com/sched-ext/scx/blob/e8e68e8ee80f65f62a6e900d457306217b764e58/scheds/rust/scx_lavd/src/main.rs#L794 > > These progs are not executed by sched-ext core, > so not really sched-ext progs. > They're auxiliary progs that populate configs and knobs in bpf maps > that sched-ext progs use later. > >> >>>> As BPF_PROG_TYPE_SYSCALL becomes more general, it would be valuable to >>>> make more kfuncs available for BPF_PROG_TYPE_SYSCALL. >>> >>> Maybe. I still don't understand how it helps CRIB goal. >> >> For CRIB goals, the program type is not important. What is important is >> that CRIB bpf programs are able to call the required kfuncs, and that >> CRIB ebpf programs can be executed from userspace. >> >> In our previous discussion, the conclusion was that we do not need a >> separate CRIB program type [1]. >> >> BPF_PROG_TYPE_SYSCALL can be executed from userspace via prog_run, which >> fits the CRIB use case of calling the ebpf program from userspace to get >> process information. >> >> So BPF_PROG_TYPE_SYSCALL becomes an option. >> >> [1]: >> https://lore.kernel.org/bpf/etzm4h5qm2jhgi6d4pevooy2sebrvgb3lsa67ym4x7zbh5bgnj@feoli4hj22so/ >> >> In fs/bpf_fs_kfuncs.c, CRIB currently needs bpf_fget_task (dump files >> opened by the process), bpf_put_file, and bpf_get_task_exe_file. >> >> So I would like these kfuncs to be available for BPF_PROG_TYPE_SYSCALL. >> >> bpf_get_dentry_xattr, bpf_get_file_xattr, and bpf_path_d_path have >> nothing to do with CRIB, but they are all in bpf_fs_kfunc_set_ids. >> >> Should we make bpf_fs_kfunc_set_ids available to BPF_PROG_TYPE_SYSCALL >> as a whole? Or create a separate set? Maybe we can discuss. > > I don't think it's necessary to slide and dice that match. > Since they're all safe from syscall prog it's cleaner to enable them all. > > When I said: > >> I still don't understand how it helps CRIB goal. > > I meant how are you going to use them from CRIB ? > > Patch 5 selftest does: > > + file = bpf_fget_task(task, test_fd1); > + if (file == NULL) { > + err = 2; > + return 0; > + } > + > + if (file->f_op != &pipefifo_fops) { > + err = 3; > + bpf_put_file(file); > + return 0; > + } > + > + bpf_put_file(file); > > > It's ok for selftest, but not enough to explain the motivation and > end-to-end operation of CRIB. > > Patch 2 selftest is also weak. > It's not using bpf_iter_task_file_next() like iterators are > normally used. > > When selftests are basic sanity tests, it begs the question: what's next? > How are they going to be used for real? In my plan CRIB ebpf program will look like this SEC("syscall") int dump_task_files(struct task_arg *arg) ... SEC("syscall") int dump_task_socket(struct socket_arg *arg) ... SEC("syscall") int dump_task_pipe(struct pipe_arg *arg) ... Since the complexity of an ebpf program is limited, I am unable to implement dumping all types of files within one ebpf program, so I need to provide separate bpf programs for different file types (restoring part is similar). In dump_task_files will use bpf_iter_task_file to obtain the file descriptor, the file type (socket, pipe, ...) and other necessary information of all files opened by the process. And then the userspace program will pass the file descriptors to different dump_task_xxx ebpf programs based on the different file types. bpf_fget_task will be used in the dump_task_xxx ebpf programs. In restoring part, ebpf program is used only as minimal necessary help, and most of the restoring part continue to use the original kernel interface. For example, when restoring sockets, we still use the original kernel interface "socket" to create sockets, and only use the ebpf program when necessary (e.g. to set TCP internal states). Thanks to BPF CO-RE, it would be more convenient to extend the data structure of kfuncs than the data structure of the system call interface, so this still has value. Once I have finished a good enough solution, I will send out this part of the patch series (as soon as I can, even though I cannot do it full time). All other patch series related to opened 'files' (socket, pipe, ...) depend on bpf_iter_task_file and bpf_fget_task.
On Tue, Dec 10, 2024 at 10:58:52AM -0800, Alexei Starovoitov wrote: > On Tue, Dec 10, 2024 at 6:43 AM Christian Brauner <brauner@kernel.org> wrote: > > > > On Tue, Dec 10, 2024 at 02:03:53PM +0000, Juntong Deng 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 and TRACING > > > program types. > > > > I would like a detailed explanation from the maintainers what it means > > to make this available to SYSCALL program types, please. > > Sigh. Hm? Was that directed at my question? I don't have the background to judge this and this whole api looks like a giant footgun so far for questionable purposes. I have a hard time seeing parts of CRIU moved into bpf especially because all of the userspace stuff exists. > This is obviously not safe from tracing progs. > > From BPF_PROG_TYPE_SYSCALL these kfuncs should be safe to use, > since those progs are not attached to anything. > Such progs can only be executed via sys_bpf syscall prog_run command. > They're sleepable, preemptable, faultable, in task ctx. > > But I'm not sure what's the value of enabling these kfuncs for > BPF_PROG_TYPE_SYSCALL.
On 2024/12/17 12:30, Christian Brauner wrote: > On Tue, Dec 10, 2024 at 10:58:52AM -0800, Alexei Starovoitov wrote: >> On Tue, Dec 10, 2024 at 6:43 AM Christian Brauner <brauner@kernel.org> wrote: >>> >>> On Tue, Dec 10, 2024 at 02:03:53PM +0000, Juntong Deng 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 and TRACING >>>> program types. >>> >>> I would like a detailed explanation from the maintainers what it means >>> to make this available to SYSCALL program types, please. >> >> Sigh. > > Hm? Was that directed at my question? I don't have the background to > judge this and this whole api looks like a giant footgun so far for > questionable purposes. > > I have a hard time seeing parts of CRIU moved into bpf especially > because all of the userspace stuff exists. > All these kfuncs I want to add are not CRIB specific but generic. Although these kfuncs are to be added because of CRIB, these kfuncs are essentially to give BPF the ability to access process-related information. Obtaining process-related information is not a requirement specific to checkpoint/restore scenarios, but is also required in other scenarios. Access to process-related information is a generic ability that would be useful for scenarios other than checkpoint/restore. Therefore these generic kfuncs will be valuable to all BPF users in the long run. >> This is obviously not safe from tracing progs. >> >> From BPF_PROG_TYPE_SYSCALL these kfuncs should be safe to use, >> since those progs are not attached to anything. >> Such progs can only be executed via sys_bpf syscall prog_run command. >> They're sleepable, preemptable, faultable, in task ctx. >> >> But I'm not sure what's the value of enabling these kfuncs for >> BPF_PROG_TYPE_SYSCALL.
diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c index 19a9d45c47f9..609d6b2af1db 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,23 +178,18 @@ BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS) BTF_ID_FLAGS(func, bpf_fget_task, KF_ACQUIRE | KF_TRUSTED_ARGS | KF_RET_NULL) 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) - return 0; - return -EACCES; -} - static const struct btf_kfunc_id_set bpf_fs_kfunc_set = { .owner = THIS_MODULE, .set = &bpf_fs_kfunc_set_ids, - .filter = bpf_fs_kfuncs_filter, }; 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); + ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_fs_kfunc_set); + return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &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; -} - char _license[] SEC("license") = "GPL";
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 and TRACING program types. Signed-off-by: Juntong Deng <juntong.deng@outlook.com> --- fs/bpf_fs_kfuncs.c | 21 +++++-------------- .../selftests/bpf/progs/verifier_vfs_reject.c | 10 --------- 2 files changed, 5 insertions(+), 26 deletions(-)