[PATCH] NFS: possible NULL pointer deref in nfs_sillyrename()

From: Jesper Juhl
Date: Wed Aug 16 2006 - 18:18:51 EST

The coverity checker spotted this as bug #1013.

If we get a NULL dentry->d_inode, then regardless of
if (dentry->d_flags & DCACHE_NFSFS_RENAMED)
turns out to be false we'll end up dereferencing
that NULL d_inode in two places below.

And since the check for "(!dentry->d_inode)" even exists
(although inside #ifdef NFS_PARANOIA) I take that to mean
that this is a possibility.
And the fact that we check
if (dentry->d_flags & DCACHE_NFSFS_RENAMED)
must also mean that there are cases where the check could

So, we can get in trouble here :

1) as an arg to sprintf() :
sprintf(silly, ".nfs%*.*lx",
i_inosize, i_inosize, dentry->d_inode->i_ino);

2) or we pass it to nfs_inode_return_delegation() which then dereferences it.

I propose the following patch to handle this.

Compile tested only.

If this patch is somehow incorrect I'd appreciate an explanation of why :)

Signed-off-by: Jesper Juhl <jesper.juhl@xxxxxxxxx>

fs/nfs/dir.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)

--- linux-2.6.18-rc4-orig/fs/nfs/dir.c 2006-08-11 00:11:12.000000000 +0200
+++ linux-2.6.18-rc4/fs/nfs/dir.c 2006-08-17 00:06:23.000000000 +0200
@@ -1299,15 +1299,15 @@ static int nfs_sillyrename(struct inode
nfs_inc_stats(dir, NFSIOS_SILLYRENAME);

-if (!dentry->d_inode)
-printk("NFS: silly-renaming %s/%s, negative dentry??\n",
-dentry->d_parent->d_name.name, dentry->d_name.name);
+ error = -EBUSY;
+ if (!dentry->d_inode) {
+ printk("NFS: silly-renaming %s/%s, negative dentry??\n",
+ dentry->d_parent->d_name.name, dentry->d_name.name);
+ goto out;
+ }
* We don't allow a dentry to be silly-renamed twice.
- error = -EBUSY;
if (dentry->d_flags & DCACHE_NFSFS_RENAMED)
goto out;

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/