Re: [PATCH RFC] proc: Don't retain negative dentries

[Al Viro <viro@ZenIV.linux.org.uk>]

On Mon, Oct 08, 2018 at 07:02:09PM +0200, Ahmad Fatoum wrote:
> Hello,
> 
> On 10/8/18 6:55 PM, Al Viro wrote:
> > 
> > What the hell does that have to do with negative dentries anywhere???
> 
> It's possible that this needs fixing at another place. I don't know,
> but this seems to work for me, that's why I prefixed with RFC.

OK, to elaborate: where have you seen negative dentries on procfs in
the first place?  I'm trying to find a way for such to happen, but
I don't see any.  And in any case, these ->d_delete() and ->d_revalidate()
instances would've been oopsing on such.

->d_delete() is about retaining _unused_ dentries in hash for future lookups;
nothing to do with positive/negative.  *And* ->d_delete() is called only when
refcount hits zero.  If another process opens the damn thing and keeps it opened,
->d_delete() won't be called at all and your patch won't change the behaviour
of the entire thing.

If anything, you might want to have separate ->d_op for /proc/*/net, so
that its ->d_revalidate() would return 0 if netns doesn't match.  Would
need a way to keep some information allowing to detect the switchover, of
course (either in PROC_I(inode) or in ->d_fsdata of that dentry - in the
latter case you'd want to do whatever you need to dispose of that in
->d_release()).  Check in revalidate should be along the lines of "do what's
currently done in get_proc_task_net(), compare the result with the memorized
value, bugger off on mismatch", perhaps with memorized value being
counted as a reference (in which case you'd want to do put_net() when
disposing of the inode or dentry, whichever you use to keep it in).  In
that case proc_tgid_net_lookup()/proc_tgid_net_getattr()/proc_tgid_net_readdir()
would simply use the stored reference instead of messing with get_proc_task_net()
and put_net().

You'd need separate dentry_operations for /proc/*/net and /proc/*/*/net,
instead of using pid_dentry_operations.  That would need to be recognized
in proc_pident_instantiate() (_without_ memcmp(p->name, "net", 4) on each
call of that thing, preferably).  I'd put that new instance of dentry_operations
(along with the methods in it, of course) into fs/proc/proc_net.c, where
we already have the inode and file methods of /proc/*/net.