Virtiofsd pull 2020-03-03

xattr fixes from Misono. -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEERfXHG0oMt/uXep+pBRYzHrxb/ecFAl5edHgACgkQBRYzHrxb /efp/hAAr48at6AwqxhW0e/tALyPT40LT9N88NOT1qyKWMdAPz8b6lM0P21wAgvf t4gfidPuwN7ySP5bnYUUJZPIe12z1FLxqiKLgB8vaUZxrVl8C3xE/HkqmQyqxOiv vn50WKKgv4lKVw/gX8JW9+occSeRIkLgDrOWE7rDtaCk7SkEzlbM83S4VVLfIbtD leRG/p0XoqF1z/nL8+X4pTx2TzJG5E02jWURrtWgS5O2MCzk0Axtq5L9qGdza2iK Fzpn6P0X5NW7pk0O4dZLxP70scVpSm7mTPVPcVGbFkMem2bQJh+bIVS6Q4uPl7Kc pANuFvnPSbaK1BeUGAHZ0ATbecBvBMk6Ysmf1oQNycs9xwhPErSoqtiF8V52uuqe mce7xKKAz9FZ1aGDWj81u5V69xeGtpu1GncdDgJo9A30EOXgJZzuEpL689+IBeKY xFcfrA0jWT4H4BOAjJ+hfhKYw9UD4OrozbY81z2U/sQg8nUyoEoZorgLe7YrjrXO 4obTWBW4fH7O7aqpCKjyqJ2t/YsPrCL9nnWSlsx7LhD9npYywkfxupD/WTsS1Meh voetOecgtP2bpeA55NampMS8JrHz7fnY6OEE3fpirVvMRCOLkEtxWNKlvLtGC2Te SoVv9IW2XOX2dmWJEHXUCr0XbgCQPStC6yqHkUhPtpfXAuDT3Iw= =arLT -----END PGP SIGNATURE----- Merge remote-tracking branch 'remotes/dgilbert-gitlab/tags/pull-virtiofs-20200303' into staging Virtiofsd pull 2020-03-03 xattr fixes from Misono. # gpg: Signature made Tue 03 Mar 2020 15:15:04 GMT # gpg: using RSA key 45F5C71B4A0CB7FB977A9FA90516331EBC5BFDE7 # gpg: Good signature from "Dr. David Alan Gilbert (RH2) <dgilbert@redhat.com>" [full] # Primary key fingerprint: 45F5 C71B 4A0C B7FB 977A 9FA9 0516 331E BC5B FDE7 * remotes/dgilbert-gitlab/tags/pull-virtiofs-20200303: virtiofsd: Fix xattr operations virtiofsd: passthrough_ll: cleanup getxattr/listxattr Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2024-11-05 20:35:44 +00:00 · 2020-03-03 15:20:12 +00:00 · 2020-03-03 15:20:12 +00:00 · abfa865014
commit abfa865014
parent 104933c4a9 bdfd667883
3 changed files with 89 additions and 69 deletions
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@ -426,6 +426,8 @@ err:
    return ret;
 }

+static __thread bool clone_fs_called;
+
 /* Process one FVRequest in a thread pool */
 static void fv_queue_worker(gpointer data, gpointer user_data)
 {
@ -441,6 +443,17 @@ static void fv_queue_worker(gpointer data, gpointer user_data)

    assert(se->bufsize > sizeof(struct fuse_in_header));

+    if (!clone_fs_called) {
+        int ret;
+
+        /* unshare FS for xattr operation */
+        ret = unshare(CLONE_FS);
+        /* should not fail */
+        assert(ret == 0);
+
+        clone_fs_called = true;
+    }
+
    /*
     * An element contains one request and the space to send our response
     * They're spread over multiple descriptors in a scatter/gather set
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@ -123,7 +123,7 @@ struct lo_inode {
    pthread_mutex_t plock_mutex;
    GHashTable *posix_locks; /* protected by lo_inode->plock_mutex */

-    bool is_symlink;
+    mode_t filetype;
 };

 struct lo_cred {
@ -695,7 +695,7 @@ static int utimensat_empty(struct lo_data *lo, struct lo_inode *inode,
    struct lo_inode *parent;
    char path[PATH_MAX];

-    if (inode->is_symlink) {
+    if (S_ISLNK(inode->filetype)) {
        res = utimensat(inode->fd, "", tv, AT_EMPTY_PATH);
        if (res == -1 && errno == EINVAL) {
            /* Sorry, no race free way to set times on symlink. */
@ -928,7 +928,8 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
            goto out_err;
        }

-        inode->is_symlink = S_ISLNK(e->attr.st_mode);
+        /* cache only filetype */
+        inode->filetype = (e->attr.st_mode & S_IFMT);

        /*
         * One for the caller and one for nlookup (released in
@ -1135,7 +1136,7 @@ static int linkat_empty_nofollow(struct lo_data *lo, struct lo_inode *inode,
    struct lo_inode *parent;
    char path[PATH_MAX];

-    if (inode->is_symlink) {
+    if (S_ISLNK(inode->filetype)) {
        res = linkat(inode->fd, "", dfd, name, AT_EMPTY_PATH);
        if (res == -1 && (errno == ENOENT || errno == EINVAL)) {
            /* Sorry, no race free way to hard-link a symlink. */
@ -2189,40 +2190,43 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
    fuse_log(FUSE_LOG_DEBUG, "lo_getxattr(ino=%" PRIu64 ", name=%s size=%zd)\n",
             ino, name, size);

-    if (inode->is_symlink) {
-        /* Sorry, no race free way to getxattr on symlink. */
-        saverr = EPERM;
-        goto out;
-    }
-
-    sprintf(procname, "%i", inode->fd);
-    fd = openat(lo->proc_self_fd, procname, O_RDONLY);
-    if (fd < 0) {
-        goto out_err;
-    }
-
    if (size) {
        value = malloc(size);
        if (!value) {
            goto out_err;
        }
+    }

-        ret = fgetxattr(fd, name, value, size);
-        if (ret == -1) {
+    sprintf(procname, "%i", inode->fd);
+    /*
+     * It is not safe to open() non-regular/non-dir files in file server
+     * unless O_PATH is used, so use that method for regular files/dir
+     * only (as it seems giving less performance overhead).
+     * Otherwise, call fchdir() to avoid open().
+     */
+    if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) {
+        fd = openat(lo->proc_self_fd, procname, O_RDONLY);
+        if (fd < 0) {
            goto out_err;
        }
+        ret = fgetxattr(fd, name, value, size);
+    } else {
+        /* fchdir should not fail here */
+        assert(fchdir(lo->proc_self_fd) == 0);
+        ret = getxattr(procname, name, value, size);
+        assert(fchdir(lo->root.fd) == 0);
+    }
+
+    if (ret == -1) {
+        goto out_err;
+    }
+    if (size) {
        saverr = 0;
        if (ret == 0) {
            goto out;
        }
-
        fuse_reply_buf(req, value, ret);
    } else {
-        ret = fgetxattr(fd, name, NULL, 0);
-        if (ret == -1) {
-            goto out_err;
-        }
-
        fuse_reply_xattr(req, ret);
    }
 out_free:
@ -2238,7 +2242,6 @@ out_free:
 out_err:
    saverr = errno;
 out:
-    lo_inode_put(lo, &inode);
    fuse_reply_err(req, saverr);
    goto out_free;
 }
@ -2267,40 +2270,37 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size)
    fuse_log(FUSE_LOG_DEBUG, "lo_listxattr(ino=%" PRIu64 ", size=%zd)\n", ino,
             size);

-    if (inode->is_symlink) {
-        /* Sorry, no race free way to listxattr on symlink. */
-        saverr = EPERM;
-        goto out;
-    }
-
-    sprintf(procname, "%i", inode->fd);
-    fd = openat(lo->proc_self_fd, procname, O_RDONLY);
-    if (fd < 0) {
-        goto out_err;
-    }
-
    if (size) {
        value = malloc(size);
        if (!value) {
            goto out_err;
        }
+    }

-        ret = flistxattr(fd, value, size);
-        if (ret == -1) {
+    sprintf(procname, "%i", inode->fd);
+    if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) {
+        fd = openat(lo->proc_self_fd, procname, O_RDONLY);
+        if (fd < 0) {
            goto out_err;
        }
+        ret = flistxattr(fd, value, size);
+    } else {
+        /* fchdir should not fail here */
+        assert(fchdir(lo->proc_self_fd) == 0);
+        ret = listxattr(procname, value, size);
+        assert(fchdir(lo->root.fd) == 0);
+    }
+
+    if (ret == -1) {
+        goto out_err;
+    }
+    if (size) {
        saverr = 0;
        if (ret == 0) {
            goto out;
        }
-
        fuse_reply_buf(req, value, ret);
    } else {
-        ret = flistxattr(fd, NULL, 0);
-        if (ret == -1) {
-            goto out_err;
-        }
-
        fuse_reply_xattr(req, ret);
    }
 out_free:
@ -2316,7 +2316,6 @@ out_free:
 out_err:
    saverr = errno;
 out:
-    lo_inode_put(lo, &inode);
    fuse_reply_err(req, saverr);
    goto out_free;
 }
@ -2345,20 +2344,21 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
    fuse_log(FUSE_LOG_DEBUG, "lo_setxattr(ino=%" PRIu64
             ", name=%s value=%s size=%zd)\n", ino, name, value, size);

-    if (inode->is_symlink) {
-        /* Sorry, no race free way to setxattr on symlink. */
-        saverr = EPERM;
-        goto out;
-    }
-
    sprintf(procname, "%i", inode->fd);
-    fd = openat(lo->proc_self_fd, procname, O_RDWR);
-    if (fd < 0) {
-        saverr = errno;
-        goto out;
+    if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) {
+        fd = openat(lo->proc_self_fd, procname, O_RDONLY);
+        if (fd < 0) {
+            saverr = errno;
+            goto out;
+        }
+        ret = fsetxattr(fd, name, value, size, flags);
+    } else {
+        /* fchdir should not fail here */
+        assert(fchdir(lo->proc_self_fd) == 0);
+        ret = setxattr(procname, name, value, size, flags);
+        assert(fchdir(lo->root.fd) == 0);
    }

-    ret = fsetxattr(fd, name, value, size, flags);
    saverr = ret == -1 ? errno : 0;

 out:
@ -2393,20 +2393,21 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *name)
    fuse_log(FUSE_LOG_DEBUG, "lo_removexattr(ino=%" PRIu64 ", name=%s)\n", ino,
             name);

-    if (inode->is_symlink) {
-        /* Sorry, no race free way to setxattr on symlink. */
-        saverr = EPERM;
-        goto out;
-    }
-
    sprintf(procname, "%i", inode->fd);
-    fd = openat(lo->proc_self_fd, procname, O_RDWR);
-    if (fd < 0) {
-        saverr = errno;
-        goto out;
+    if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) {
+        fd = openat(lo->proc_self_fd, procname, O_RDONLY);
+        if (fd < 0) {
+            saverr = errno;
+            goto out;
+        }
+        ret = fremovexattr(fd, name);
+    } else {
+        /* fchdir should not fail here */
+        assert(fchdir(lo->proc_self_fd) == 0);
+        ret = removexattr(procname, name);
+        assert(fchdir(lo->root.fd) == 0);
    }

-    ret = fremovexattr(fd, name);
    saverr = ret == -1 ? errno : 0;

 out:
@ -2806,7 +2807,7 @@ static void setup_root(struct lo_data *lo, struct lo_inode *root)
        exit(1);
    }

-    root->is_symlink = false;
+    root->filetype = S_IFDIR;
    root->fd = fd;
    root->key.ino = stat.st_ino;
    root->key.dev = stat.st_dev;
--- a/tools/virtiofsd/seccomp.c
+++ b/tools/virtiofsd/seccomp.c
@ -41,6 +41,7 @@ static const int syscall_whitelist[] = {
    SCMP_SYS(exit),
    SCMP_SYS(exit_group),
    SCMP_SYS(fallocate),
+    SCMP_SYS(fchdir),
    SCMP_SYS(fchmodat),
    SCMP_SYS(fchownat),
    SCMP_SYS(fcntl),
@ -62,7 +63,9 @@ static const int syscall_whitelist[] = {
    SCMP_SYS(getpid),
    SCMP_SYS(gettid),
    SCMP_SYS(gettimeofday),
+    SCMP_SYS(getxattr),
    SCMP_SYS(linkat),
+    SCMP_SYS(listxattr),
    SCMP_SYS(lseek),
    SCMP_SYS(madvise),
    SCMP_SYS(mkdirat),
@ -85,6 +88,7 @@ static const int syscall_whitelist[] = {
    SCMP_SYS(recvmsg),
    SCMP_SYS(renameat),
    SCMP_SYS(renameat2),
+    SCMP_SYS(removexattr),
    SCMP_SYS(rt_sigaction),
    SCMP_SYS(rt_sigprocmask),
    SCMP_SYS(rt_sigreturn),
@ -98,10 +102,12 @@ static const int syscall_whitelist[] = {
    SCMP_SYS(setresuid32),
 #endif
    SCMP_SYS(set_robust_list),
+    SCMP_SYS(setxattr),
    SCMP_SYS(symlinkat),
    SCMP_SYS(time), /* Rarely needed, except on static builds */
    SCMP_SYS(tgkill),
    SCMP_SYS(unlinkat),
+    SCMP_SYS(unshare),
    SCMP_SYS(utimensat),
    SCMP_SYS(write),
    SCMP_SYS(writev),