All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH RFC] namei: don't drop link paths acquired under LOOKUP_RCU
Date: Sun, 14 Feb 2021 16:05:22 +0000	[thread overview]
Message-ID: <YClKQlivsPPcbyCd@zeniv-ca.linux.org.uk> (raw)
In-Reply-To: <8b114189-e943-a7e6-3d31-16aa8a148da6@kernel.dk>

On Sun, Feb 07, 2021 at 01:26:19PM -0700, Jens Axboe wrote:

> Al, not sure if this is the right fix for the situation, but it's
> definitely a problem. Observed by doing a LOOKUP_CACHED of something with
> links, using /proc/self/comm as the example in the attached way to
> demonstrate this problem.

That's definitely not the right fix.  What your analysis has missed is
what legitimize_links() does to nd->depth when called.  IOW, on transitions
from RCU mode you want nd->depth to set according the number of links we'd
grabbed references to.  Flatly setting it to 0 on failure exit will lead
to massive leaks.

Could you check if the following fixes your reproducers?

diff --git a/fs/namei.c b/fs/namei.c
index 4cae88733a5c..afb293b39be7 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -687,7 +687,7 @@ static bool try_to_unlazy(struct nameidata *nd)
 
 	nd->flags &= ~LOOKUP_RCU;
 	if (nd->flags & LOOKUP_CACHED)
-		goto out1;
+		goto out2;
 	if (unlikely(!legitimize_links(nd)))
 		goto out1;
 	if (unlikely(!legitimize_path(nd, &nd->path, nd->seq)))
@@ -698,6 +698,8 @@ static bool try_to_unlazy(struct nameidata *nd)
 	BUG_ON(nd->inode != parent->d_inode);
 	return true;
 
+out2:
+	nd->depth = 0;	// as we hadn't gotten to legitimize_links()
 out1:
 	nd->path.mnt = NULL;
 	nd->path.dentry = NULL;
@@ -725,7 +727,7 @@ static bool try_to_unlazy_next(struct nameidata *nd, struct dentry *dentry, unsi
 
 	nd->flags &= ~LOOKUP_RCU;
 	if (nd->flags & LOOKUP_CACHED)
-		goto out2;
+		goto out3;
 	if (unlikely(!legitimize_links(nd)))
 		goto out2;
 	if (unlikely(!legitimize_mnt(nd->path.mnt, nd->m_seq)))
@@ -753,6 +755,8 @@ static bool try_to_unlazy_next(struct nameidata *nd, struct dentry *dentry, unsi
 	rcu_read_unlock();
 	return true;
 
+out3:
+	nd->depth = 0;	// as we hadn't gotten to legitimize_links()
 out2:
 	nd->path.mnt = NULL;
 out1:

  reply	other threads:[~2021-02-14 16:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-07 20:26 [PATCH RFC] namei: don't drop link paths acquired under LOOKUP_RCU Jens Axboe
2021-02-14 16:05 ` Al Viro [this message]
2021-02-14 16:40   ` Al Viro
2021-02-14 16:45     ` Jens Axboe
2021-02-14 22:57       ` Al Viro
2021-02-15  3:31         ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YClKQlivsPPcbyCd@zeniv-ca.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=axboe@kernel.dk \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.