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 ?