Re: [PATCH] proc_sysctl: Fix up ->is_seen() handling

[NeilBrown]

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