NFS: guard against confused server in nfs_atomic_open()
authorNeilBrown <neilb@suse.com>
Mon, 3 Jul 2017 05:27:26 +0000 (15:27 +1000)
committerAnna Schumaker <Anna.Schumaker@Netapp.com>
Thu, 13 Jul 2017 20:00:08 +0000 (16:00 -0400)
A confused server could return a filehandle for an
NFSv4 OPEN request, which it previously returned for a directory.
So the inode returned by  ->open_context() in nfs_atomic_open()
could conceivably be a directory inode.

This has particular implications for the call to
nfs_file_set_open_context() in nfs_finish_open().
If that is called on a directory inode, then the nfs_open_context
that gets stored in the filp->private_data will be linked to
nfs_inode->open_files.

When the directory is closed, nfs_closedir() will (ultimately)
free the ->private_data, but not unlink it from nfs_inode->open_files
(because it doesn't expect an nfs_open_context there).

Subsequently the memory could get used for something else and eventually
if the ->open_files list is walked, the walker will fall off the end and
crash.

So: change nfs_finish_open() to only call nfs_file_set_open_context()
for regular-file inodes.

This failure mode has been seen in a production setting (unknown NFS
server implementation).  The kernel was v3.0 and the specific sequence
seen would not affect more recent kernels, but I think a risk is still
present, and caution is wise.

Signed-off-by: NeilBrown <neilb@suse.com>
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
fs/nfs/dir.c

index 98b18aa..90bc402 100644 (file)
@@ -1431,8 +1431,10 @@ static int nfs_finish_open(struct nfs_open_context *ctx,
        err = finish_open(file, dentry, do_open, opened);
        if (err)
                goto out;
-       nfs_file_set_open_context(file, ctx);
-
+       if (S_ISREG(file->f_path.dentry->d_inode->i_mode))
+               nfs_file_set_open_context(file, ctx);
+       else
+               err = -ESTALE;
 out:
        return err;
 }