From b07581d2d5add23ae163e3fbfb2fa5d36076922f Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 5 Oct 2018 14:29:46 -0400 Subject: [PATCH 1/3] cachefiles: fix the race between cachefiles_bury_object() and rmdir(2) the victim might've been rmdir'ed just before the lock_rename(); unlike the normal callers, we do not look the source up after the parents are locked - we know it beforehand and just recheck that it's still the child of what used to be its parent. Unfortunately, the check is too weak - we don't spot a dead directory since its ->d_parent is unchanged, dentry is positive, etc. So we sail all the way to ->rename(), with hosting filesystems _not_ expecting to be asked renaming an rmdir'ed subdirectory. The fix is easy, fortunately - the lock on parent is sufficient for making IS_DEADDIR() on child safe. Cc: stable@vger.kernel.org Fixes: 9ae326a69004 (CacheFiles: A cache that backs onto a mounted filesystem) Signed-off-by: Al Viro --- fs/cachefiles/namei.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c index af2b17b21b94..95983c744164 100644 --- a/fs/cachefiles/namei.c +++ b/fs/cachefiles/namei.c @@ -343,7 +343,7 @@ try_again: trap = lock_rename(cache->graveyard, dir); /* do some checks before getting the grave dentry */ - if (rep->d_parent != dir) { + if (rep->d_parent != dir || IS_DEADDIR(d_inode(rep))) { /* the entry was probably culled when we dropped the parent dir * lock */ unlock_rename(cache->graveyard, dir); From 74dd7c97ea2ab08b41925ab2f472db573accda89 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 9 Oct 2018 23:32:41 -0400 Subject: [PATCH 2/3] ecryptfs_rename(): verify that lower dentries are still OK after lock_rename() We get lower layer dentries, find their parents, do lock_rename() and proceed to vfs_rename(). However, we do not check that dentries still have the same parents and are not unlinked. Need to check that... Signed-off-by: Al Viro --- fs/ecryptfs/inode.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 49121e5a8de2..5c36ceecb5c1 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -593,11 +593,16 @@ ecryptfs_rename(struct inode *old_dir, struct dentry *old_dentry, lower_new_dir_dentry = dget_parent(lower_new_dentry); target_inode = d_inode(new_dentry); trap = lock_rename(lower_old_dir_dentry, lower_new_dir_dentry); - /* source should not be ancestor of target */ - if (trap == lower_old_dentry) { - rc = -EINVAL; + rc = -EINVAL; + if (lower_old_dentry->d_parent != lower_old_dir_dentry) + goto out_lock; + if (lower_new_dentry->d_parent != lower_new_dir_dentry) + goto out_lock; + if (d_unhashed(lower_old_dentry) || d_unhashed(lower_new_dentry)) + goto out_lock; + /* source should not be ancestor of target */ + if (trap == lower_old_dentry) goto out_lock; - } /* target should not be ancestor of source */ if (trap == lower_new_dentry) { rc = -ENOTEMPTY; From 3df629d873f8683af6f0d34dfc743f637966d483 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 13 Oct 2018 00:19:13 -0400 Subject: [PATCH 3/3] gfs2_meta: ->mount() can get NULL dev_name get in sync with mount_bdev() handling of the same Reported-by: syzbot+c54f8e94e6bba03b04e9@syzkaller.appspotmail.com Cc: stable@vger.kernel.org Signed-off-by: Al Viro --- fs/gfs2/ops_fstype.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c index c2469833b4fb..6b84ef6ccff3 100644 --- a/fs/gfs2/ops_fstype.c +++ b/fs/gfs2/ops_fstype.c @@ -1333,6 +1333,9 @@ static struct dentry *gfs2_mount_meta(struct file_system_type *fs_type, struct path path; int error; + if (!dev_name || !*dev_name) + return ERR_PTR(-EINVAL); + error = kern_path(dev_name, LOOKUP_FOLLOW, &path); if (error) { pr_warn("path_lookup on %s returned error %d\n",