All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Dave Jones <davej@redhat.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: [CFT] Re: VFS deadlock ?
Date: Fri, 22 Mar 2013 04:37:15 +0000	[thread overview]
Message-ID: <20130322043715.GL21522@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20130322014037.GK21522@ZenIV.linux.org.uk>

On Fri, Mar 22, 2013 at 01:40:37AM +0000, Al Viro wrote:

> Yeah, I went to do such patch after sending the previous mail and noticed
> that we already did it that way.  Simplicity of error recovery was probably
> more important consideration there - I honestly don't remember the reasoning
> in such details; it had been a decade or so...  So lock_rename() doing
> ->d_inode comparison (with dire comment re not expecting that to be sufficient
> for anything other than this bug in procfs) will probably suffice for fs/namei.c
> part of it; I'm still looking at dcache.c side of things...

FWIW, a relatively crude solution is this:

diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 4b3b3ff..778cbac 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -416,8 +416,7 @@ struct dentry *proc_lookup_de(struct proc_dir_entry *de, struct inode *dir,
 			if (!inode)
 				return ERR_PTR(-ENOMEM);
 			d_set_d_op(dentry, &proc_dentry_operations);
-			d_add(dentry, inode);
-			return NULL;
+			return d_materialise_unique(dentry, inode);
 		}
 	}
 	spin_unlock(&proc_subdir_lock);

It *is* crude, but it restores the assert, killing the deadlock and lets
everything work more or less as it used to.  The case where things start
to look odd is this:

root@kvm-amd64:~# cd /proc/1/net/stat/; ls /proc/2/net/stat; /bin/pwd
arp_cache  ndisc_cache  rt_cache
/proc/2/net/stat

IOW, if we were about to create a directory alias, the old dentry gets moved
in new place.  OTOH, I think it's the most robust backportable variant we
can do.  And yes, that should apply at least all the way back to 2.6.25 when
Eric acked a patch from Pavel that really should've been nacked...

Folks, could you test that one and see if any real userland breaks on that?
If everything works, I'd propose that one for -stable.

  reply	other threads:[~2013-03-22  4:37 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-21 19:06 VFS deadlock ? Dave Jones
2013-03-21 19:21 ` Al Viro
2013-03-21 20:31   ` Dave Jones
2013-03-21 19:29 ` Al Viro
2013-03-21 20:15   ` Linus Torvalds
2013-03-21 20:26     ` Dave Jones
2013-03-21 20:32       ` Linus Torvalds
2013-03-21 20:36         ` Dave Jones
2013-03-21 20:47           ` Al Viro
2013-03-21 21:02             ` Dave Jones
2013-03-21 21:18               ` Linus Torvalds
2013-03-21 21:26                 ` Al Viro
2013-03-21 21:41                   ` Dave Jones
2013-03-21 21:47                     ` Linus Torvalds
2013-03-21 21:55                       ` Al Viro
2013-03-21 21:57                         ` Linus Torvalds
2013-03-21 22:03                           ` Al Viro
2013-03-21 21:52                     ` Al Viro
2013-03-21 22:12                 ` Dave Jones
2013-03-21 22:29                   ` Dave Jones
2013-03-21 22:53                   ` Linus Torvalds
2013-03-21 23:07                     ` Dave Jones
2013-03-21 23:36                     ` Al Viro
2013-03-21 23:58                       ` Linus Torvalds
2013-03-22  0:01                         ` Linus Torvalds
2013-03-22  0:12                           ` Al Viro
2013-03-22  0:20                             ` Al Viro
2013-03-22  0:22                             ` Linus Torvalds
2013-03-22  1:22                               ` Al Viro
2013-03-22  1:33                                 ` Linus Torvalds
2013-03-22  1:40                                   ` Al Viro
2013-03-22  4:37                                     ` Al Viro [this message]
2013-03-22  4:55                                       ` [CFT] " Linus Torvalds
2013-03-22  5:18                                         ` Al Viro
2013-03-22  5:33                                           ` Linus Torvalds
2013-03-22  6:09                                             ` Al Viro
2013-03-22  6:22                                               ` Al Viro
2013-03-22 16:23                                             ` Dave Jones
2013-03-22 19:43                                             ` Linus Torvalds
2013-03-22 21:28                                               ` Al Viro
2013-03-22 22:57                                               ` Eric W. Biederman
2013-03-22  5:19                                         ` Linus Torvalds
2013-03-22  0:08                         ` Al Viro
2013-03-22  0:15                           ` Linus Torvalds
2013-03-22  0:19                             ` Linus Torvalds

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=20130322043715.GL21522@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=davej@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.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.