> The problem isn't the call into the filesystem - it's may_lookup() > passing the inode to security modules where we dereference > inode->i_security and assume that it is valid for the life of the > object access being made. > > That's my point - if we have a lookup race and the inode is being > destroyed at this point (i.e. I_FREEING is set) inode->i_security > *is not valid* and should not be accessed by *anyone*. It's valid. The inode_free_security hooks unlist or deregister the context as e.g., selinux_inode_free_security() does. It's fine to access inode->i_security after the individual hooks have been called; before or within the delayed freeing of i_security. And selinux and bpf are the only cases where deregistration happens while also implementing or in the case of bpf allowing to implement security_inode_permission(). > If we decide that every pathwalk accessed object attached to the > inode needs to be RCU freed, then __destroy_inode() is the wrong > place to be freeing them - i_callback() (the call_rcu() inode The superblock might already be gone by the time free_inode() is called. And stuff like selinux accesses the superblock from inode->i_sb during security_inode_free_security(). So moving it in there isn't an option. It needs to be called before. If one wanted it in there the obvious way to do it would be to split deregistration and freeing into two hooks security_inode_deregister() and then move the rcu part of security_inode_free() into i_callback so it gets wasted together with the inode. But i_callback() isn't called for xfs and so the way it's currently done is so far the best.
