| Message ID | 162993585927.7591.10174443410031404560@noble.neil.brown.name (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Don't block writes to swap-files with ETXTBSY. | expand |
On Thu, Aug 26, 2021 at 09:57:39AM +1000, NeilBrown wrote: > > Commit dc617f29dbe5 ("vfs: don't allow writes to swap files") > broke swap-over-NFS as it introduced an ETXTBSY error when NFS tries to > swap-out using ->direct_IO(). > > There is no sound justification for this error. File permissions are > sufficient to stop non-root users from writing to a swap file, and root > must always be cautious not to do anything dangerous. > > These checks effectively provide a mandatory write lock on swap, and > mandatory locks are not supported in Linux. > > So remove all the checks that return ETXTBSY when attempts are made to > write to swap. Swap files are not just any files and do need a mandatory write lock as they are part of the kernel VM and writing to them will mess up the kernel badly. David Howells actually has sent various patches to fix swap over NFS in the last weeks.
On Sat, 28 Aug 2021, Christoph Hellwig wrote: > On Thu, Aug 26, 2021 at 09:57:39AM +1000, NeilBrown wrote: > > > > Commit dc617f29dbe5 ("vfs: don't allow writes to swap files") > > broke swap-over-NFS as it introduced an ETXTBSY error when NFS tries to > > swap-out using ->direct_IO(). > > > > There is no sound justification for this error. File permissions are > > sufficient to stop non-root users from writing to a swap file, and root > > must always be cautious not to do anything dangerous. > > > > These checks effectively provide a mandatory write lock on swap, and > > mandatory locks are not supported in Linux. > > > > So remove all the checks that return ETXTBSY when attempts are made to > > write to swap. > > Swap files are not just any files and do need a mandatory write lock > as they are part of the kernel VM and writing to them will mess up > the kernel badly. David Howells actually has sent various patches > to fix swap over NFS in the last weeks. > > There are lots of different things root can do which will mess up the kernel badly. The backing-store can still be changed through some other means. Do you have a particular threat or risk scenario other than "root might get careless"? Yes, I've seen David's patches. I posted this one because I think the original patch which broke swap-over-NFS was not just unfortunate, but wrong. Permissions are how we protect files, not ETXTBSY. NeilBrown
On Sat, Aug 28, 2021 at 09:10:15AM +1000, NeilBrown wrote: > There are lots of different things root can do which will mess up the > kernel badly. The backing-store can still be changed through some other > means. > Do you have a particular threat or risk scenario other than "root might > get careless"? No, it is just that scenario. But one that is much easier to trigger than more convoluted ways for a root user to trigger damage through device files, and one that can't be prevented through LSMs or the capability system.
diff --git a/fs/block_dev.c b/fs/block_dev.c index 9ef4f1fc2cb0..e5605c88e6e9 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1623,9 +1623,6 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) if (bdev_read_only(I_BDEV(bd_inode))) return -EPERM; - if (IS_SWAPFILE(bd_inode) && !is_hibernate_resume_dev(bd_inode->i_rdev)) - return -ETXTBSY; - if (!iov_iter_count(from)) return 0; diff --git a/fs/read_write.c b/fs/read_write.c index 9db7adf160d2..14f1c622a066 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1442,9 +1442,6 @@ static int generic_copy_file_checks(struct file *file_in, loff_t pos_in, if (IS_IMMUTABLE(inode_out)) return -EPERM; - if (IS_SWAPFILE(inode_in) || IS_SWAPFILE(inode_out)) - return -ETXTBSY; - /* Ensure offsets don't wrap. */ if (pos_in + count < pos_in || pos_out + count < pos_out) return -EOVERFLOW; @@ -1646,9 +1643,6 @@ ssize_t generic_write_checks(struct kiocb *iocb, struct iov_iter *from) loff_t count; int ret; - if (IS_SWAPFILE(inode)) - return -ETXTBSY; - if (!iov_iter_count(from)) return 0;
Commit dc617f29dbe5 ("vfs: don't allow writes to swap files") broke swap-over-NFS as it introduced an ETXTBSY error when NFS tries to swap-out using ->direct_IO(). There is no sound justification for this error. File permissions are sufficient to stop non-root users from writing to a swap file, and root must always be cautious not to do anything dangerous. These checks effectively provide a mandatory write lock on swap, and mandatory locks are not supported in Linux. So remove all the checks that return ETXTBSY when attempts are made to write to swap. Fixes: dc617f29dbe5 ("vfs: don't allow writes to swap files") Signed-off-by: NeilBrown <neilb@suse.de> --- fs/block_dev.c | 3 --- fs/read_write.c | 6 ------ 2 files changed, 9 deletions(-)