Messages in this thread |  | | Date | Fri, 5 Oct 2018 17:22:20 -0700 | From | Alexei Starovoitov <> | Subject | Re: [PATCH bpf-next 1/6] bpf: introduce BPF_PROG_TYPE_FILE_FILTER |
| |
On Sat, Oct 06, 2018 at 12:47:06AM +0100, Al Viro wrote: > > And no, it's not that each of those filesystems does not use inode->i_ino at all. > There's a bunch of library helpers in fs/*.c that happen to use the value filesystem > has seen fit to store there. Whether to use those helpers or not, what to store in > that field, etc. is, again, entirely up to the filesystem in question. > generic_fillattr() is one of those, that's all there is to it.
makes sense.
> That's precisely why I really do not like the idea of hooks poking in the internals > of kernel data structures. Especially since not even "it's not visible outside of > a subsystem-internal header" appears to slow you down.
that's why there is a code review to get the patches in shape that don't expose kernel internals by _accident_.
> The same goes for tracepoints, etc. - turning random details of implementation into > a carved in stone ABI is actively harmful.
we're not talking about tracepoints here.
> PS: If anything, visibility to hooks should be opt-in.
not sure how do you expect 'opt-in' to be imlemented. As far as exposing inode and dev I agree that was a wrong approach. Going to switch to struct file_handle instead. Doing statx for every file_open is probably too slow for practical use.
> Sure, we can start actively hiding > the things, but that's a winless arms race - even if we bloody went and encrypted the > private stuff, you'd still be able to pull decryption key from where it would be accessed > by legitimate users, etc. Нахуй нам это надо?
why do this ? the use cases are explained in commit log.
Back to internals exposure. Do you see an issue in the following: struct bpf_file_info { __u32 fs_magic; // file->f_inode->i_sb->s_magic __u32 mnt_id; // real_mount(file->f_path.mnt)->mnt_id __u32 nlink; // file->f_inode->i_nlink __u32 mode; // file->f_inode->i_mode __u32 flags; // file->f_flags }; and separate helper that returns struct file_handle ? and ctx rewriting moved to fs/bpf_file_filter.c ?
|  |