On Fri, 13 Jun 2025, Al Viro wrote: > On Fri, Jun 13, 2025 at 02:54:21AM +0100, Al Viro wrote: > > On Fri, Jun 13, 2025 at 10:37:58AM +1000, NeilBrown wrote: > > > > > > Some sysctl tables can provide an is_seen() function which reports if > > > the sysctl should be visible to the current process. This is currently > > > used to cause d_compare to fail for invisible sysctls. > > > > > > This technique might have worked in 2.6.26 when it was implemented, but > > > it cannot work now. In particular if ->d_compare always fails for a > > > particular name, then d_alloc_parallel() will always create a new dentry > > > and pass it to lookup() resulting in a new inode for every lookup. I > > > tested this by changing sysctl_is_seen() to always return 0. When > > > all sysctls were still visible and repeated lookups (ls -li) reported > > > different inode numbers. > > > > What do you mean, "name"? > > The whole fucking point of that thing is that /proc/sys/net contents for > processes in different netns is not the same. And such processes should > not screw each other into the ground by doing lookups in that area. > > Yes, it means multiple children of the same dentry having the same name > *and* staying hashed at the same time. > If two threads in the same namespace look up the same name at the same time (which previously didn't exist), they will both enter d_alloc_parallel() where neither will notice the other, so both will create and install d_in_lookup() dentries, and then both will call ->lookup, creating two identical inodes. I suspect that isn't fatal, but it does seem odd. Maybe proc_sys_compare should return 0 for d_in_lookup() (aka !inode) dentries, and then proc_sys_revalidate() can perform the is_seen test and return -EAGAIN if needed, and __lookup_slow() and others could interpret that as meaning to "goto again" without calling d_invalidate(). Maybe. NeilBrown
