| Message ID | 149905964664.6926.2427823210014981309.stgit@noble (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 2ac00bf4ecf1..4f0706bd387f 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1427,8 +1427,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; }
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> --- fs/nfs/dir.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html