Hi Christian, James and Jan, > On Nov 14, 2024, at 1:49 PM, James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: [...] >> >> We can address this with something like following: >> >> #ifdef CONFIG_SECURITY >> void *i_security; >> #elif CONFIG_BPF_SYSCALL >> struct bpf_local_storage __rcu *i_bpf_storage; >> #endif >> >> This will help catch all misuse of the i_bpf_storage at compile >> time, as i_bpf_storage doesn't exist with CONFIG_SECURITY=y. >> >> Does this make sense? > > Got to say I'm with Casey here, this will generate horrible and failure > prone code. > > Since effectively you're making i_security always present anyway, > simply do that and also pull the allocation code out of security.c in a > way that it's always available? That way you don't have to special > case the code depending on whether CONFIG_SECURITY is defined. > Effectively this would give everyone a generic way to attach some > memory area to an inode. I know it's more complex than this because > there are LSM hooks that run from security_inode_alloc() but if you can > make it work generically, I'm sure everyone will benefit. On a second thought, I think making i_security generic is not the right solution for "BPF inode storage in tracing use cases". This is because i_security serves a very specific use case: it points to a piece of memory whose size is calculated at system boot time. If some of the supported LSMs is not enabled by the lsm= kernel arg, the kernel will not allocate memory in i_security for them. The only way to change lsm= is to reboot the system. BPF LSM programs can be disabled at the boot time, which fits well in i_security. However, BPF tracing programs cannot be disabled at boot time (even we change the code to make it possible, we are not likely to disable BPF tracing). IOW, as long as CONFIG_BPF_SYSCALL is enabled, we expect some BPF tracing programs to load at some point of time, and these programs may use BPF inode storage. Therefore, with CONFIG_BPF_SYSCALL enabled, some extra memory always will be attached to i_security (maybe under a different name, say, i_generic) of every inode. In this case, we should really add i_bpf_storage directly to the inode, because another pointer jump via i_generic gives nothing but overhead. Does this make sense? Or did I misunderstand the suggestion? Thanks, Song
