[PATCH 0/2] btrfs: fix races between exec and defrag

[PATCH 0/2] btrfs: fix races between exec and defrag

[Adam Borowski]

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]

Hi!
Here's a patch to fix ETXTBSY races between defrag and exec -- similar to
what was just submitted for dedupe, even to the point of being followed by
a second patch that replaces EINVAL with EPERM.

As defrag is not something you're going to do on files you don't write, I
skipped complex rules and I'm sending the original version of the patch
as-is.  It has stewed in my tree for two years (long story...), tested on
multiple machines.

Attached: a simple tool to fragment a file, by ten O_SYNC rewrites of length
1 at random positions; racey vs concurrent writes or execs but shouldn't
damage the file otherwise.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢰⠒⠀⣿⡁ 
⢿⡄⠘⠷⠚⠋⠀ Certified airhead; got the CT scan to prove that!
⠈⠳⣄⠀⠀⠀⠀ 

[-- Attachment #2: fragme.c --]
[-- Type: text/x-csrc, Size: 1363 bytes --]

#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdarg.h>
#include <errno.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>
#include <unistd.h>
#include <sys/syscall.h>

static void die(const char *txt, ...) __attribute__((format (printf, 1, 2)));
static void die(const char *txt, ...)
{
    fprintf(stderr, "fragme: ");

    va_list ap;
    va_start(ap, txt);
    vfprintf(stderr, txt, ap);
    va_end(ap);

    exit(1);
}

static uint64_t rnd(uint64_t max)
{
    __uint128_t r;
    if (syscall(SYS_getrandom, &r, sizeof(r), 0)==-1)
        die("getrandom(): %m\n");
    return r%max;
}

int main(int argc, char **argv)
{
    if (argc!=2)
        die("Usage: fragme <file>\n");

    int fd = open(argv[1], O_RDWR|O_SYNC);
    if (fd == -1)
        die("open(\"%s\"): %m\n", argv[1]);
    off_t size = lseek(fd, 0, SEEK_END);
    if (size == -1)
        die("lseek(SEEK_END): %m\n");

    for (int i=0; i<10; ++i)
    {
        off_t off = rnd(size);
        char b;
        if (lseek(fd, off, SEEK_SET) != off)
            die("lseek for read: %m\n");
        if (read(fd, &b, 1) != 1)
            die("read(%lu): %m\n", off);
        if (lseek(fd, off, SEEK_SET) != off)
            die("lseek for write: %m\n");
        if (write(fd, &b, 1) != 1)
            die("write: %m\n");
    }

    return 0;
}

[PATCH 1/2] btrfs: allow defrag on a file opened ro that has rw permissions

[Adam Borowski]

Requiring a rw descriptor conflicts both ways with exec, returning ETXTBSY
whenever you try to defrag a program that's currently being run, or
causing intermittent exec failures on a live system being defragged.

As defrag doesn't change the file's contents in any way, there's no reason
to consider it a rw operation.  Thus, let's check only whether the file
could have been opened rw.  Such access control is still needed as
currently defrag can use extra disk space, and might trigger bugs.

Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
 fs/btrfs/ioctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 632e26d6f7ce..b75db9d72106 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2561,7 +2561,8 @@ static int btrfs_ioctl_defrag(struct file *file, void __user *argp)
 		ret = btrfs_defrag_root(root);
 		break;
 	case S_IFREG:
-		if (!(file->f_mode & FMODE_WRITE)) {
+		if (!capable(CAP_SYS_ADMIN) &&
+		    inode_permission(inode, MAY_WRITE)) {
 			ret = -EINVAL;
 			goto out;
 		}
-- 
2.17.0

[PATCH 2/2] btrfs: defrag: return EPERM not EINVAL when only permissions fail

[Adam Borowski]

We give EINVAL when the request is invalid; here it's ok but merely the
user has insufficient privileges.  Thus, this return value reflects the
error better -- as discussed in the identical case for dedupe.

According to codesearch.debian.net, no userspace program distinguishes
these values beyond strerror().

Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
 fs/btrfs/ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index b75db9d72106..ae6a110987a7 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2563,7 +2563,7 @@ static int btrfs_ioctl_defrag(struct file *file, void __user *argp)
 	case S_IFREG:
 		if (!capable(CAP_SYS_ADMIN) &&
 		    inode_permission(inode, MAY_WRITE)) {
-			ret = -EINVAL;
+			ret = -EPERM;
 			goto out;
 		}
 
-- 
2.17.0

[PATCH] defrag: open files RO

[Adam Borowski]

NOT FOR MERGING -- requires kernel versioning

Fixes EXTXBSY races.

Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
 cmds-filesystem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 30a50bf5..7eb6b7bb 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -876,7 +876,7 @@ static int defrag_callback(const char *fpath, const struct stat *sb,
 	if ((typeflag == FTW_F) && S_ISREG(sb->st_mode)) {
 		if (defrag_global_verbose)
 			printf("%s\n", fpath);
-		fd = open(fpath, O_RDWR);
+		fd = open(fpath, O_RDONLY);
 		if (fd < 0) {
 			goto error;
 		}
@@ -1012,7 +1012,7 @@ static int cmd_filesystem_defrag(int argc, char **argv)
 		int defrag_err = 0;
 
 		dirstream = NULL;
-		fd = open_file_or_dir(argv[i], &dirstream);
+		fd = open_file_or_dir3(argv[i], &dirstream, O_RDONLY);
 		if (fd < 0) {
 			error("cannot open %s: %m", argv[i]);
 			ret = -errno;
-- 
2.17.0