Re: [PATCH bpf-next 1/6] bpf: introduce BPF_PROG_TYPE_FILE_FILTER

[Alexei Starovoitov]

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 ?