vfs: allow copy_file_range from a swapfile
Subsystems (7)
PATCH 1/1 vfs: allow copy_file_range from a swapfile
read(2) is allowed from a swapfile, so copy_file_range(2) should be allowed as well.
---
Darrick,
This fixes the generic/554 issue reported by Ted.
I intend to remove the test case of copy from swap file from generic/554, so test is expected to pass with or without this fix. But if you wait for next week's xfstests update before applying this fix, then at lease maintainer that run current xfstests master could use current copy-file-range-fixes branch to pass the tests.
Thanks,
Amir.
mm/filemap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/filemap.c +1 -1
@@ -3081,7 +3081,7 @@ 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)) +if (IS_SWAPFILE (inode_out))return -ETXTBSY ;/* Ensure offsets don't wrap. */
Signature · 1 line
2.17.1
Quoted 11 lines · Amir Goldstein
read(2) is allowed from a swapfile, so
copy_file_range(2)should be allowed as well.Reported-by Theodore Ts'o <[email protected]>Fixes 96e6e8f4a68d ("vfs: add missing checks to copy_file_range")Signed-off-by Amir Goldstein <[email protected]>---
Darrick,
This fixes the generic/554 issue reported by Ted.
Frankly I think we should go the other way -- non-root doesn't get to copy from or read from swap files.
--D
Quoted 28 lines
I intend to remove the test case of copy from swap file from generic/554, so test is expected to pass with or without this fix. But if you wait for next week's xfstests update before applying this fix, then at lease maintainer that run current xfstests master could use current copy-file-range-fixes branch to pass the tests.
Thanks,
Amir.mm/filemap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)mm/filemap.c +1
-1@@ -3081,7 +3081,7 @@ 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)) +if (IS_SWAPFILE (inode_out))return -ETXTBSY ;/* Ensure offsets don't wrap. */ Signature · 1 line
2.17.1
Quoted 9 lines · Darrick J. Wong
Quoted 8 lines · Amir Goldstein
read(2) is allowed from a swapfile, so
copy_file_range(2)should be allowed as well.Reported-by Theodore Ts'o <[email protected]>Fixes 96e6e8f4a68d ("vfs: add missing checks to copy_file_range")Signed-off-by Amir Goldstein <[email protected]>---
Quoted 6 lines
Quoted 3 lines
Darrick,
This fixes the generic/554 issue reported by Ted.
Frankly I think we should go the other way -- non-root doesn't get to copy from or read from swap files.
The issue is that without this patch, *root* doesn't get to copy from swap files. Non-root shouldn't have access via Unix permissions. We could add a special case if we don't trust system administrators to be able to set the Unix permissions correctly, I suppose, but we don't do that for block devices when they are mounted....
- Ted
Quoted 12 lines · Theodore Ts'o
Quoted 11 lines · Darrick J. Wong
Quoted 10 lines · Amir Goldstein
read(2) is allowed from a swapfile, so
copy_file_range(2)should be allowed as well.Reported-by Theodore Ts'o <[email protected]>Fixes 96e6e8f4a68d ("vfs: add missing checks to copy_file_range")Signed-off-by Amir Goldstein <[email protected]>---
Darrick,
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
This fixes the generic/554 issue reported by Ted.
Frankly I think we should go the other way -- non-root doesn't get to copy from or read from swap files.
The issue is that without this patch, *root* doesn't get to copy from swap files. Non-root shouldn't have access via Unix permissions. We
I'm not sure even root should have that privilege - it's a swap file, and until you swapoff, it's owned by the kernel and we shouldn't let backup programs copy your swapped out credit card numbers onto tape.
Quoted 3 lines
could add a special case if we don't trust system administrators to be able to set the Unix permissions correctly, I suppose, but we don't do that for block devices when they are mounted....
...and administrators often mkfs over mounted filesystems because we let them read and write block devices. Granted I tried to fix that once and LVM totally stopped working...
--D
Quoted 2 lines
- Ted
Quoted 3 lines · Darrick J. Wong
...and administrators often mkfs over mounted filesystems because we let them read and write block devices. Granted I tried to fix that once and LVM totally stopped working...
That must have been a long time ago. We now overload O_EXCL on blockdevice to only allow a single exclusive openers, and mounted filesystems as well as raid drivers have been setting set for at leat 10 years.
Quoted 19 lines · Darrick J. Wong
Quoted 17 lines · Theodore Ts'o
Quoted 15 lines · Darrick J. Wong
Quoted 11 lines · Amir Goldstein
read(2) is allowed from a swapfile, so
copy_file_range(2)should be allowed as well.Reported-by Theodore Ts'o <[email protected]>Fixes 96e6e8f4a68d ("vfs: add missing checks to copy_file_range")Signed-off-by Amir Goldstein <[email protected]>---
Darrick,
This fixes the generic/554 issue reported by Ted.
Frankly I think we should go the other way -- non-root doesn't get to copy from or read from swap files.
Quoted 7 lines
Quoted 2 lines
The issue is that without this patch, *root* doesn't get to copy from swap files. Non-root shouldn't have access via Unix permissions. We
I'm not sure even root should have that privilege - it's a swap file, and until you swapoff, it's owned by the kernel and we shouldn't let backup programs copy your swapped out credit card numbers onto tape.
I am not a security expert and I do not want to be, but I believe it's better to have a complete security model before plugging random "security holes".
That said. I don't have a strong feeling about allowing copy_file_range from swap file. If someone complains and they have a valid use case, we can always relax it then.
Anyway, as you saw, I removed the test case from xfstest, leaving the behavior (as far as the testsuite cares) undefined.
Thanks,
Amir.