Commit 51cc3a66 authored by Hugh Dickins's avatar Hugh Dickins Committed by Linus Torvalds

fs, mm: fix race in unlinking swapfile

We had a recurring situation in which admin procedures setting up
swapfiles would race with test preparation clearing away swapfiles; and
just occasionally that got stuck on a swapfile "(deleted)" which could
never be swapped off.  That is not supposed to be possible.

2.6.28 commit f9454548 ("don't unlink an active swapfile") admitted
that it was leaving a race window open: now close it.

may_delete() makes the IS_SWAPFILE check (amongst many others) before
inode_lock has been taken on target: now repeat just that simple check in
vfs_unlink() and vfs_rename(), after taking inode_lock.

Which goes most of the way to fixing the race, but swapon() must also
check after it acquires inode_lock, that the file just opened has not
already been unlinked.

Link: https://lkml.kernel.org/r/e17b91ad-a578-9a15-5e3-4989e0f999b5@google.com
Fixes: f9454548 ("don't unlink an active swapfile")
Signed-off-by: default avatarHugh Dickins <hughd@google.com>
Reviewed-by: default avatarJan Kara <jack@suse.cz>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 9857a17f
...@@ -4024,7 +4024,9 @@ int vfs_unlink(struct user_namespace *mnt_userns, struct inode *dir, ...@@ -4024,7 +4024,9 @@ int vfs_unlink(struct user_namespace *mnt_userns, struct inode *dir,
return -EPERM; return -EPERM;
inode_lock(target); inode_lock(target);
if (is_local_mountpoint(dentry)) if (IS_SWAPFILE(target))
error = -EPERM;
else if (is_local_mountpoint(dentry))
error = -EBUSY; error = -EBUSY;
else { else {
error = security_inode_unlink(dir, dentry); error = security_inode_unlink(dir, dentry);
...@@ -4526,6 +4528,10 @@ int vfs_rename(struct renamedata *rd) ...@@ -4526,6 +4528,10 @@ int vfs_rename(struct renamedata *rd)
else if (target) else if (target)
inode_lock(target); inode_lock(target);
error = -EPERM;
if (IS_SWAPFILE(source) || (target && IS_SWAPFILE(target)))
goto out;
error = -EBUSY; error = -EBUSY;
if (is_local_mountpoint(old_dentry) || is_local_mountpoint(new_dentry)) if (is_local_mountpoint(old_dentry) || is_local_mountpoint(new_dentry))
goto out; goto out;
......
...@@ -3130,6 +3130,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) ...@@ -3130,6 +3130,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
struct filename *name; struct filename *name;
struct file *swap_file = NULL; struct file *swap_file = NULL;
struct address_space *mapping; struct address_space *mapping;
struct dentry *dentry;
int prio; int prio;
int error; int error;
union swap_header *swap_header; union swap_header *swap_header;
...@@ -3173,6 +3174,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) ...@@ -3173,6 +3174,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
p->swap_file = swap_file; p->swap_file = swap_file;
mapping = swap_file->f_mapping; mapping = swap_file->f_mapping;
dentry = swap_file->f_path.dentry;
inode = mapping->host; inode = mapping->host;
error = claim_swapfile(p, inode); error = claim_swapfile(p, inode);
...@@ -3180,6 +3182,10 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) ...@@ -3180,6 +3182,10 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
goto bad_swap; goto bad_swap;
inode_lock(inode); inode_lock(inode);
if (d_unlinked(dentry) || cant_mount(dentry)) {
error = -ENOENT;
goto bad_swap_unlock_inode;
}
if (IS_SWAPFILE(inode)) { if (IS_SWAPFILE(inode)) {
error = -EBUSY; error = -EBUSY;
goto bad_swap_unlock_inode; goto bad_swap_unlock_inode;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment