diff mbox series

proc: fix dentry/inode overinstantiating under /proc/${pid}/net

Message ID YjdVHgildbWO7diJ@localhost.localdomain (mailing list archive)
State New, archived
Headers show
Series proc: fix dentry/inode overinstantiating under /proc/${pid}/net | expand

Commit Message

Alexey Dobriyan March 20, 2022, 4:23 p.m. UTC
When a process exits, /proc/${pid}, and /proc/${pid}/net dentries are flushed.
However some leaf dentries like /proc/${pid}/net/arp_cache aren't.
That's because respective PDEs have proc_misc_d_revalidate() hook which
returns 1 and leaves dentries/inodes in the LRU.

Force revalidation/lookup on everything under /proc/${pid}/net by inheriting
proc_net_dentry_ops.

Fixes: c6c75deda813 ("proc: fix lookup in /proc/net subdirectories after setns(2)")
Reported-by: hui li <juanfengpy@gmail.com>
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 fs/proc/generic.c  |    4 ++++
 fs/proc/proc_net.c |    3 +++
 2 files changed, 7 insertions(+)

Comments

cael March 21, 2022, 9:15 a.m. UTC | #1
proc_misc_dentry_ops is a general ops for dentry under /proc, except
for "/proc/${pid}/net",other dentries may also use there own ops too,
so I think change proc_misc_d_delete may be better?
see patch under: https://lkml.org/lkml/2022/3/17/319

Alexey Dobriyan <adobriyan@gmail.com> 于2022年3月21日周一 00:24写道:
>
> When a process exits, /proc/${pid}, and /proc/${pid}/net dentries are flushed.
> However some leaf dentries like /proc/${pid}/net/arp_cache aren't.
> That's because respective PDEs have proc_misc_d_revalidate() hook which
> returns 1 and leaves dentries/inodes in the LRU.
>
> Force revalidation/lookup on everything under /proc/${pid}/net by inheriting
> proc_net_dentry_ops.
>
> Fixes: c6c75deda813 ("proc: fix lookup in /proc/net subdirectories after setns(2)")
> Reported-by: hui li <juanfengpy@gmail.com>
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
>
>  fs/proc/generic.c  |    4 ++++
>  fs/proc/proc_net.c |    3 +++
>  2 files changed, 7 insertions(+)
>
> --- a/fs/proc/generic.c
> +++ b/fs/proc/generic.c
> @@ -448,6 +448,10 @@ static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent,
>         proc_set_user(ent, (*parent)->uid, (*parent)->gid);
>
>         ent->proc_dops = &proc_misc_dentry_ops;
> +       /* Revalidate everything under /proc/${pid}/net */
> +       if ((*parent)->proc_dops == &proc_net_dentry_ops) {
> +               pde_force_lookup(ent);
> +       }
>
>  out:
>         return ent;
> --- a/fs/proc/proc_net.c
> +++ b/fs/proc/proc_net.c
> @@ -376,6 +376,9 @@ static __net_init int proc_net_ns_init(struct net *net)
>
>         proc_set_user(netd, uid, gid);
>
> +       /* Seed dentry revalidation for /proc/${pid}/net */
> +       pde_force_lookup(netd);
> +
>         err = -EEXIST;
>         net_statd = proc_net_mkdir(net, "stat", netd);
>         if (!net_statd)
Alexey Dobriyan March 21, 2022, 10:40 a.m. UTC | #2
On Mon, Mar 21, 2022 at 05:15:02PM +0800, hui li wrote:
> Alexey Dobriyan <adobriyan@gmail.com> 于2022年3月21日周一 00:24写道:
> >
> > When a process exits, /proc/${pid}, and /proc/${pid}/net dentries are flushed.
> > However some leaf dentries like /proc/${pid}/net/arp_cache aren't.
> > That's because respective PDEs have proc_misc_d_revalidate() hook which
> > returns 1 and leaves dentries/inodes in the LRU.
> >
> > Force revalidation/lookup on everything under /proc/${pid}/net by inheriting
> > proc_net_dentry_ops.
> >
> > Fixes: c6c75deda813 ("proc: fix lookup in /proc/net subdirectories after setns(2)")
> > Reported-by: hui li <juanfengpy@gmail.com>
> > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> > ---
> >
> >  fs/proc/generic.c  |    4 ++++
> >  fs/proc/proc_net.c |    3 +++
> >  2 files changed, 7 insertions(+)
> >
> > --- a/fs/proc/generic.c
> > +++ b/fs/proc/generic.c
> > @@ -448,6 +448,10 @@ static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent,
> >         proc_set_user(ent, (*parent)->uid, (*parent)->gid);
> >
> >         ent->proc_dops = &proc_misc_dentry_ops;
> > +       /* Revalidate everything under /proc/${pid}/net */
> > +       if ((*parent)->proc_dops == &proc_net_dentry_ops) {
> > +               pde_force_lookup(ent);
> > +       }
> >
> >  out:
> >         return ent;
> > --- a/fs/proc/proc_net.c
> > +++ b/fs/proc/proc_net.c
> > @@ -376,6 +376,9 @@ static __net_init int proc_net_ns_init(struct net *net)
> >
> >         proc_set_user(netd, uid, gid);
> >
> > +       /* Seed dentry revalidation for /proc/${pid}/net */
> > +       pde_force_lookup(netd);
> > +
> >         err = -EEXIST;
> >         net_statd = proc_net_mkdir(net, "stat", netd);
> >         if (!net_statd)

> proc_misc_dentry_ops is a general ops for dentry under /proc, except
> for "/proc/${pid}/net",other dentries may also use there own ops too,
> so I think change proc_misc_d_delete may be better?
> see patch under: https://lkml.org/lkml/2022/3/17/319

I don't think so.

proc_misc_d_delete covers "everything else" part under /proc/ and
/proc/net which are 2 separate trees. Now /proc/net/ requires
revalidation because of

	commit c6c75deda81344c3a95d1d1f606d5cee109e5d54
	proc: fix lookup in /proc/net subdirectories after setns(2)

so the bug is that the above commit was applied only partially.
In particular, /proc/*/net/stat/arp_cache was created with
proc_create_seq_data(), avoiding proc_net_* APIs.

And there is probably the same "lookup after setns find wrong file"
if you search hard enough in /proc/*/net/

This is the logic. Please test on your systems.
cael March 21, 2022, 11:50 a.m. UTC | #3
I see, I will test on my system.

Alexey Dobriyan <adobriyan@gmail.com> 于2022年3月21日周一 18:40写道:
>
> On Mon, Mar 21, 2022 at 05:15:02PM +0800, hui li wrote:
> > Alexey Dobriyan <adobriyan@gmail.com> 于2022年3月21日周一 00:24写道:
> > >
> > > When a process exits, /proc/${pid}, and /proc/${pid}/net dentries are flushed.
> > > However some leaf dentries like /proc/${pid}/net/arp_cache aren't.
> > > That's because respective PDEs have proc_misc_d_revalidate() hook which
> > > returns 1 and leaves dentries/inodes in the LRU.
> > >
> > > Force revalidation/lookup on everything under /proc/${pid}/net by inheriting
> > > proc_net_dentry_ops.
> > >
> > > Fixes: c6c75deda813 ("proc: fix lookup in /proc/net subdirectories after setns(2)")
> > > Reported-by: hui li <juanfengpy@gmail.com>
> > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> > > ---
> > >
> > >  fs/proc/generic.c  |    4 ++++
> > >  fs/proc/proc_net.c |    3 +++
> > >  2 files changed, 7 insertions(+)
> > >
> > > --- a/fs/proc/generic.c
> > > +++ b/fs/proc/generic.c
> > > @@ -448,6 +448,10 @@ static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent,
> > >         proc_set_user(ent, (*parent)->uid, (*parent)->gid);
> > >
> > >         ent->proc_dops = &proc_misc_dentry_ops;
> > > +       /* Revalidate everything under /proc/${pid}/net */
> > > +       if ((*parent)->proc_dops == &proc_net_dentry_ops) {
> > > +               pde_force_lookup(ent);
> > > +       }
> > >
> > >  out:
> > >         return ent;
> > > --- a/fs/proc/proc_net.c
> > > +++ b/fs/proc/proc_net.c
> > > @@ -376,6 +376,9 @@ static __net_init int proc_net_ns_init(struct net *net)
> > >
> > >         proc_set_user(netd, uid, gid);
> > >
> > > +       /* Seed dentry revalidation for /proc/${pid}/net */
> > > +       pde_force_lookup(netd);
> > > +
> > >         err = -EEXIST;
> > >         net_statd = proc_net_mkdir(net, "stat", netd);
> > >         if (!net_statd)
>
> > proc_misc_dentry_ops is a general ops for dentry under /proc, except
> > for "/proc/${pid}/net",other dentries may also use there own ops too,
> > so I think change proc_misc_d_delete may be better?
> > see patch under: https://lkml.org/lkml/2022/3/17/319
>
> I don't think so.
>
> proc_misc_d_delete covers "everything else" part under /proc/ and
> /proc/net which are 2 separate trees. Now /proc/net/ requires
> revalidation because of
>
>         commit c6c75deda81344c3a95d1d1f606d5cee109e5d54
>         proc: fix lookup in /proc/net subdirectories after setns(2)
>
> so the bug is that the above commit was applied only partially.
> In particular, /proc/*/net/stat/arp_cache was created with
> proc_create_seq_data(), avoiding proc_net_* APIs.
>
> And there is probably the same "lookup after setns find wrong file"
> if you search hard enough in /proc/*/net/
>
> This is the logic. Please test on your systems.
diff mbox series

Patch

--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -448,6 +448,10 @@  static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent,
 	proc_set_user(ent, (*parent)->uid, (*parent)->gid);
 
 	ent->proc_dops = &proc_misc_dentry_ops;
+	/* Revalidate everything under /proc/${pid}/net */
+	if ((*parent)->proc_dops == &proc_net_dentry_ops) {
+		pde_force_lookup(ent);
+	}
 
 out:
 	return ent;
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -376,6 +376,9 @@  static __net_init int proc_net_ns_init(struct net *net)
 
 	proc_set_user(netd, uid, gid);
 
+	/* Seed dentry revalidation for /proc/${pid}/net */
+	pde_force_lookup(netd);
+
 	err = -EEXIST;
 	net_statd = proc_net_mkdir(net, "stat", netd);
 	if (!net_statd)