Hi James, Thanks for your input! > On Nov 14, 2024, at 1:49 PM, James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: [...] >> >> Actually, I can understand the concern with union. Although, >> the logic is set at kernel compile time, it is still possible >> for kernel source code to use i_bpf_storage when >> CONFIG_SECURITY is enabled. (Yes, I guess now I finally understand >> the concern). >> >> 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. Yes, as I described in another email in the thread [1], this turned out to cause more troubles than I thought. > 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? I think this is a very good idea. If folks agree with this approach, I am more than happy to draft patch for this. Thanks again, Song > 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. [1] https://lore.kernel.org/linux-fsdevel/86C65B85-8167-4D04-BFF5-40FD4F3407A4@xxxxxx/
