On Thu, Nov 14, 2024 at 4:49 PM James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > > 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. My apologies on such a delayed response to this thread, I've had very limited network access for a bit due to travel and the usual merge window related distractions (and some others that were completely unrelated) have left me with quite the mail backlog to sift through. Enough with the excuses ... Quickly skimming this thread and the v3 patchset, I would advise you that there may be issues around using BPF LSMs and storing inode LSM state outside the LSM managed inode storage blob. Beyond the conceptual objections that Casey has already mentioned, there have been issues relating to the disjoint inode and inode->i_security lifetimes. While I believe we have an okay-ish solution in place now for LSMs, I can't promise everything will work fine for BPF LSMs that manage their inode LSM state outside of the LSM managed inode blob. I'm sure you've already looked at it, but if you haven't it might be worth looking at security_inode_free() to see some of the details. In a perfect world inode->i_security would be better synchronized with the inode lifetime, but that would involve changes that the VFS folks dislike. However, while I will recommend against it, I'm not going to object to you storing BPF LSM inode state elsewhere, that is up to you and KP (he would need to ACK that as the BPF LSM maintainer). I just want you to know that if things break, there isn't much we (the LSM folks) will be able to do to help other than suggest you go back to using the LSM managed inode storage. As far as some of the other ideas in this thread are concerned, at this point in time I don't think we want to do any massive rework or consolidation around i_security. That's a critical field for the LSM framework and many individual LSMs and there is work underway which relies on this as a LSM specific inode storage blob; having to share i_security with non-LSM users or moving the management of i_security outside of the LSM is not something I'm overly excited about right now. -- paul-moore.com
