[CFT] Re: VFS deadlock ?

From: Al Viro
Date: Fri Mar 22 2013 - 00:37:28 EST


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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/