From b6853f78e763d42c7a158d8de3549c9827c604ab Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Thu, 25 Feb 2016 18:17:38 +0100 Subject: [PATCH 1/6] hpfs: don't truncate the file when delete fails The delete opration can allocate additional space on the HPFS filesystem due to btree split. The HPFS driver checks in advance if there is available space, so that it won't corrupt the btree if we run out of space during splitting. If there is not enough available space, the HPFS driver attempted to truncate the file, but this results in a deadlock since the commit 7dd29d8d865efdb00c0542a5d2c87af8c52ea6c7 ("HPFS: Introduce a global mutex and lock it on every callback from VFS"). This patch removes the code that tries to truncate the file and -ENOSPC is returned instead. If the user hits -ENOSPC on delete, he should try to delete other files (that are stored in a leaf btree node), so that the delete operation will make some space for deleting the file stored in non-leaf btree node. Reported-by: Al Viro Signed-off-by: Mikulas Patocka Cc: stable@vger.kernel.org # 2.6.39+ Signed-off-by: Al Viro --- fs/hpfs/namei.c | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/fs/hpfs/namei.c b/fs/hpfs/namei.c index 506765afa1a3..bb8d67e2740a 100644 --- a/fs/hpfs/namei.c +++ b/fs/hpfs/namei.c @@ -376,12 +376,11 @@ static int hpfs_unlink(struct inode *dir, struct dentry *dentry) struct inode *inode = d_inode(dentry); dnode_secno dno; int r; - int rep = 0; int err; hpfs_lock(dir->i_sb); hpfs_adjust_length(name, &len); -again: + err = -ENOENT; de = map_dirent(dir, hpfs_i(dir)->i_dno, name, len, &dno, &qbh); if (!de) @@ -401,33 +400,9 @@ again: hpfs_error(dir->i_sb, "there was error when removing dirent"); err = -EFSERROR; break; - case 2: /* no space for deleting, try to truncate file */ - + case 2: /* no space for deleting */ err = -ENOSPC; - if (rep++) - break; - - dentry_unhash(dentry); - if (!d_unhashed(dentry)) { - hpfs_unlock(dir->i_sb); - return -ENOSPC; - } - if (generic_permission(inode, MAY_WRITE) || - !S_ISREG(inode->i_mode) || - get_write_access(inode)) { - d_rehash(dentry); - } else { - struct iattr newattrs; - /*pr_info("truncating file before delete.\n");*/ - newattrs.ia_size = 0; - newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME; - err = notify_change(dentry, &newattrs, NULL); - put_write_access(inode); - if (!err) - goto again; - } - hpfs_unlock(dir->i_sb); - return -ENOSPC; + break; default: drop_nlink(inode); err = 0; From 0fcbf996d848d03573113d83f4e3fb3bcfa5ab5e Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Fri, 26 Feb 2016 18:53:12 +0100 Subject: [PATCH 2/6] fs: return -EOPNOTSUPP if clone is not supported -EBADF is a rather confusing error if an operations is not supported, and nfsd gets rather upset about it. Signed-off-by: Christoph Hellwig Signed-off-by: Al Viro --- fs/read_write.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fs/read_write.c b/fs/read_write.c index 0c8782aa3d71..dadf24e5c95b 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1533,10 +1533,12 @@ int vfs_clone_file_range(struct file *file_in, loff_t pos_in, if (!(file_in->f_mode & FMODE_READ) || !(file_out->f_mode & FMODE_WRITE) || - (file_out->f_flags & O_APPEND) || - !file_in->f_op->clone_file_range) + (file_out->f_flags & O_APPEND)) return -EBADF; + if (!file_in->f_op->clone_file_range) + return -EOPNOTSUPP; + ret = clone_verify_area(file_in, pos_in, len, false); if (ret) return ret; From c80567c82ae4814a41287618e315a60ecf513be6 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 27 Feb 2016 19:17:33 -0500 Subject: [PATCH 3/6] do_last(): don't let a bogus return value from ->open() et.al. to confuse us ... into returning a positive to path_openat(), which would interpret that as "symlink had been encountered" and proceed to corrupt memory, etc. It can only happen due to a bug in some ->open() instance or in some LSM hook, etc., so we report any such event *and* make sure it doesn't trick us into further unpleasantness. Cc: stable@vger.kernel.org # v3.6+, at least Signed-off-by: Al Viro --- fs/namei.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/namei.c b/fs/namei.c index f624d132e01e..e30deefad8f8 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3273,6 +3273,10 @@ opened: goto exit_fput; } out: + if (unlikely(error > 0)) { + WARN_ON(1); + error = -EINVAL; + } if (got_write) mnt_drop_write(nd->path.mnt); path_put(&save_parent); From d4565649b6d6923369112758212b851adc407f0c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 27 Feb 2016 19:23:16 -0500 Subject: [PATCH 4/6] namei: ->d_inode of a pinned dentry is stable only for positives both do_last() and walk_component() risk picking a NULL inode out of dentry about to become positive, *then* checking its flags and seeing that it's not negative anymore and using (already stale by then) value they'd fetched earlier. Usually ends up oopsing soon after that... Cc: stable@vger.kernel.org # v3.13+ Signed-off-by: Al Viro --- fs/namei.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index e30deefad8f8..e0881c0bd228 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1743,11 +1743,11 @@ static int walk_component(struct nameidata *nd, int flags) if (err < 0) return err; - inode = d_backing_inode(path.dentry); seq = 0; /* we are already out of RCU mode */ err = -ENOENT; if (d_is_negative(path.dentry)) goto out_path_put; + inode = d_backing_inode(path.dentry); } if (flags & WALK_PUT) @@ -3192,12 +3192,12 @@ retry_lookup: return error; BUG_ON(nd->flags & LOOKUP_RCU); - inode = d_backing_inode(path.dentry); seq = 0; /* out of RCU mode, so the value doesn't matter */ if (unlikely(d_is_negative(path.dentry))) { path_to_nameidata(&path, nd); return -ENOENT; } + inode = d_backing_inode(path.dentry); finish_lookup: if (nd->depth) put_link(nd); From a7f775428b8f5808815c0e3004020cedb94cbe3b Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 27 Feb 2016 19:31:01 -0500 Subject: [PATCH 5/6] should_follow_link(): validate ->d_seq after having decided to follow ... otherwise d_is_symlink() above might have nothing to do with the inode value we've got. Cc: stable@vger.kernel.org # v4.2+ Signed-off-by: Al Viro --- fs/namei.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fs/namei.c b/fs/namei.c index e0881c0bd228..65a0e9d1ea48 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1712,6 +1712,11 @@ static inline int should_follow_link(struct nameidata *nd, struct path *link, return 0; if (!follow) return 0; + /* make sure that d_is_symlink above matches inode */ + if (nd->flags & LOOKUP_RCU) { + if (read_seqcount_retry(&link->dentry->d_seq, seq)) + return -ECHILD; + } return pick_link(nd, link, inode, seq); } From 5129fa482b16615fd4464d2f5d23acb1b7056c66 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 27 Feb 2016 19:37:37 -0500 Subject: [PATCH 6/6] do_last(): ELOOP failure exit should be done after leaving RCU mode ... or we risk seeing a bogus value of d_is_symlink() there. Cc: stable@vger.kernel.org # v4.2+ Signed-off-by: Al Viro --- fs/namei.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 65a0e9d1ea48..9c590e0f66e9 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3211,11 +3211,6 @@ finish_lookup: if (unlikely(error)) return error; - if (unlikely(d_is_symlink(path.dentry)) && !(open_flag & O_PATH)) { - path_to_nameidata(&path, nd); - return -ELOOP; - } - if ((nd->flags & LOOKUP_RCU) || nd->path.mnt != path.mnt) { path_to_nameidata(&path, nd); } else { @@ -3234,6 +3229,10 @@ finish_open: return error; } audit_inode(nd->name, nd->path.dentry, 0); + if (unlikely(d_is_symlink(nd->path.dentry)) && !(open_flag & O_PATH)) { + error = -ELOOP; + goto out; + } error = -EISDIR; if ((open_flag & O_CREAT) && d_is_dir(nd->path.dentry)) goto out;