linux-kernel - Re: [3.1-rc4] vfs_rmdir() -> mutex

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20110914203441.GA10169@hostway.ca>
Date:	Wed, 14 Sep 2011 13:34:41 -0700
From:	Simon Kirby <sim@...tway.ca>
To:	Lin Ming <mlin@...pku.edu.cn>
Cc:	linux-kernel@...r.kernel.org, linux-nfs@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Fabio Coatti <fabio.coatti@...il.com>
Subject: Re: [3.1-rc4] vfs_rmdir() -> mutex_unlock() Oops

On Wed, Sep 14, 2011 at 09:48:09PM +0800, Lin Ming wrote:

> > ---[ end trace f515ec8376bdb799 ]---
> >
> > How can I further debug this? At this point, it seems to be happening several times daily.
> 
> Fabio reported a similar bug,
> 3.0.3 [BUG] unable to handle kernel NULL pointer dereference
> http://marc.info/?t=131416920900001&r=1&w=2
> 
> Do you have a test case to trigger this bug reliably?

Hello!

Sorry, I didn't realize I took this off list after replying to Linus'
resent mail, but the problem has been tracked down and fixed, and should
show up in Linus' next push.

The problem was introduced by 64252c75a2196a0cf1e0d3777143ecfe0e3ae650,
and Al Viro's patch is reproduced below.

I was able to reproduce the issue reliably with:

ssh <nfs_client> 'cd /mnt/test; mkdir foo; sleep 1; mkdir foo' ; \
ssh <nfs_server> rmdir /export/test/foo ; \
ssh <nfs_client> rmdir /mnt/test/foo

Simon-

>From Al:

[PATCH] restore pinning the victim dentry in vfs_rmdir()/vfs_rename_dir()

We used to get the victim pinned by dentry_unhash() prior to commit
0cf1e0d3777143ecfe0e3ae650 and ->rmdir()/->rename() instances relied on
that; most of them don't care, but ones that used d_delete() themselves
do.  As the result, we are getting rmdir() oopses on NFS now.

Just grab the reference before locking the victim and drop it explicitly
after unlocking, same as vfs_rename_other() does.

Signed-off-by: Al Viro <viro@...iv.linux.org.uk>
---
diff --git a/fs/namei.c b/fs/namei.c
index 5422080..22c2d5d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2619,6 +2619,7 @@ int vfs_rmdir(struct inode *dir, struct dentry *dentry)
 	if (!dir->i_op->rmdir)
 		return -EPERM;
 
+	dget(dentry);
 	mutex_lock(&dentry->d_inode->i_mutex);
 
 	error = -EBUSY;
@@ -2639,6 +2640,7 @@ int vfs_rmdir(struct inode *dir, struct dentry *dentry)
 
 out:
 	mutex_unlock(&dentry->d_inode->i_mutex);
+	dput(dentry);
 	if (!error)
 		d_delete(dentry);
 	return error;
@@ -3028,6 +3030,7 @@ static int vfs_rename_dir(struct inode *old_dir, struct dentry *old_dentry,
 	if (error)
 		return error;
 
+	dget(new_dentry);
 	if (target)
 		mutex_lock(&target->i_mutex);
 
@@ -3048,6 +3051,7 @@ static int vfs_rename_dir(struct inode *old_dir, struct dentry *old_dentry,
 out:
 	if (target)
 		mutex_unlock(&target->i_mutex);
+	dput(new_dentry);
 	if (!error)
 		if (!(old_dir->i_sb->s_type->fs_flags & FS_RENAME_DOES_D_MOVE))
 			d_move(old_dentry,new_dentry);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/