On Fri, Oct 04, 2024 at 10:46:27AM GMT, Dave Chinner wrote: > On Thu, Oct 03, 2024 at 06:17:31PM +0200, Jan Kara wrote: > > On Thu 03-10-24 23:59:51, Dave Chinner wrote: > > > As for the landlock code, I think it needs to have it's own internal > > > tracking mechanism and not search the sb inode list for inodes that > > > it holds references to. LSM cleanup should be run before before we > > > get to tearing down the inode cache, not after.... > > > > Well, I think LSM cleanup could in principle be handled together with the > > fsnotify cleanup but I didn't check the details. > > I'm not sure how we tell if an inode potentially has a LSM related > reference hanging off it. The landlock code looks to make an > assumption in that the only referenced inodes it sees will have a > valid inode->i_security pointer if landlock is enabled. i.e. it > calls landlock_inode(inode) and dereferences the returned value > without ever checking if inode->i_security is NULL or not. > > I mean, we could do a check for inode->i_security when the refcount > is elevated and replace the security_sb_delete hook with an > security_evict_inode hook similar to the proposed fsnotify eviction > from evict_inodes(). > > But screwing with LSM instructure looks .... obnoxiously complex > from the outside... Imho, please just focus on the immediate feedback and ignore all the extra bells and whistles that we could or should do. I prefer all of that to be done after this series lands.