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

[Jesper Juhl <jesper.juhl@gmail.com>]

The coverity checker spotted this as bug #1013.

If we get a NULL dentry->d_inode, then regardless of 
NFS_PARANOIA or no NFS_PARANOIA, then if 
   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 
fail.

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.
 		nfs_inode_return_delegation(dentry->d_inode);

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@gmail.com>
---

 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 
 		atomic_read(&dentry->d_count));
 	nfs_inc_stats(dir, NFSIOS_SILLYRENAME);
 
-#ifdef NFS_PARANOIA
-if (!dentry->d_inode)
-printk("NFS: silly-renaming %s/%s, negative dentry??\n",
-dentry->d_parent->d_name.name, dentry->d_name.name);
-#endif
+	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;