thebeanogamer / rpms / qemu-kvm

Forked from rpms/qemu-kvm 5 months ago
Clone

Blame SOURCES/kvm-virtiofsd-prevent-opening-of-special-files-CVE-2020-.patch

6e7d01
From cc9a776fba8ec62c862db55753107f19459dafa8 Mon Sep 17 00:00:00 2001
6e7d01
From: Jon Maloy <jmaloy@redhat.com>
6e7d01
Date: Tue, 9 Feb 2021 23:14:56 -0500
6e7d01
Subject: [PATCH 3/3] virtiofsd: prevent opening of special files
6e7d01
 (CVE-2020-35517)
6e7d01
6e7d01
RH-Author: Jon Maloy <jmaloy@redhat.com>
6e7d01
Message-id: <20210209231456.1555472-4-jmaloy@redhat.com>
6e7d01
Patchwork-id: 101023
6e7d01
O-Subject: [RHEL-8.4.0 qemu-kvm PATCH 3/3] virtiofsd: prevent opening of special files (CVE-2020-35517)
6e7d01
Bugzilla: 1919111
6e7d01
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
6e7d01
RH-Acked-by: Greg Kurz <gkurz@redhat.com>
6e7d01
RH-Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
6e7d01
6e7d01
From: Stefan Hajnoczi <stefanha@redhat.com>
6e7d01
6e7d01
A well-behaved FUSE client does not attempt to open special files with
6e7d01
FUSE_OPEN because they are handled on the client side (e.g. device nodes
6e7d01
are handled by client-side device drivers).
6e7d01
6e7d01
The check to prevent virtiofsd from opening special files is missing in
6e7d01
a few cases, most notably FUSE_OPEN. A malicious client can cause
6e7d01
virtiofsd to open a device node, potentially allowing the guest to
6e7d01
escape. This can be exploited by a modified guest device driver. It is
6e7d01
not exploitable from guest userspace since the guest kernel will handle
6e7d01
special files inside the guest instead of sending FUSE requests.
6e7d01
6e7d01
This patch fixes this issue by introducing the lo_inode_open() function
6e7d01
to check the file type before opening it. This is a short-term solution
6e7d01
because it does not prevent a compromised virtiofsd process from opening
6e7d01
device nodes on the host.
6e7d01
6e7d01
Restructure lo_create() to try O_CREAT | O_EXCL first. Note that O_CREAT
6e7d01
| O_EXCL does not follow symlinks, so O_NOFOLLOW masking is not
6e7d01
necessary here. If the file exists and the user did not specify O_EXCL,
6e7d01
open it via lo_do_open().
6e7d01
6e7d01
Reported-by: Alex Xu <alex@alxu.ca>
6e7d01
Fixes: CVE-2020-35517
6e7d01
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
6e7d01
Reviewed-by: Vivek Goyal <vgoyal@redhat.com>
6e7d01
Reviewed-by: Greg Kurz <groug@kaod.org>
6e7d01
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
6e7d01
Message-Id: <20210204150208.367837-4-stefanha@redhat.com>
6e7d01
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
6e7d01
6e7d01
(cherry picked from commit a3fdbbc7f271bff7d53d0501b29d910ece0b3789)
6e7d01
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
6e7d01
Signed-off-by: Jon Maloy <jmaloy.redhat.com>
6e7d01
---
6e7d01
 tools/virtiofsd/passthrough_ll.c | 144 ++++++++++++++++++++-----------
6e7d01
 1 file changed, 92 insertions(+), 52 deletions(-)
6e7d01
6e7d01
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
6e7d01
index e5bd3d73e4..cb0992f2db 100644
6e7d01
--- a/tools/virtiofsd/passthrough_ll.c
6e7d01
+++ b/tools/virtiofsd/passthrough_ll.c
6e7d01
@@ -535,6 +535,38 @@ static int lo_fd(fuse_req_t req, fuse_ino_t ino)
6e7d01
     return fd;
6e7d01
 }
6e7d01
 
6e7d01
+/*
6e7d01
+ * Open a file descriptor for an inode. Returns -EBADF if the inode is not a
6e7d01
+ * regular file or a directory.
6e7d01
+ *
6e7d01
+ * Use this helper function instead of raw openat(2) to prevent security issues
6e7d01
+ * when a malicious client opens special files such as block device nodes.
6e7d01
+ * Symlink inodes are also rejected since symlinks must already have been
6e7d01
+ * traversed on the client side.
6e7d01
+ */
6e7d01
+static int lo_inode_open(struct lo_data *lo, struct lo_inode *inode,
6e7d01
+                         int open_flags)
6e7d01
+{
6e7d01
+    g_autofree char *fd_str = g_strdup_printf("%d", inode->fd);
6e7d01
+    int fd;
6e7d01
+
6e7d01
+    if (!S_ISREG(inode->filetype) && !S_ISDIR(inode->filetype)) {
6e7d01
+        return -EBADF;
6e7d01
+    }
6e7d01
+
6e7d01
+    /*
6e7d01
+     * The file is a symlink so O_NOFOLLOW must be ignored. We checked earlier
6e7d01
+     * that the inode is not a special file but if an external process races
6e7d01
+     * with us then symlinks are traversed here. It is not possible to escape
6e7d01
+     * the shared directory since it is mounted as "/" though.
6e7d01
+     */
6e7d01
+    fd = openat(lo->proc_self_fd, fd_str, open_flags & ~O_NOFOLLOW);
6e7d01
+    if (fd < 0) {
6e7d01
+        return -errno;
6e7d01
+    }
6e7d01
+    return fd;
6e7d01
+}
6e7d01
+
6e7d01
 static void lo_init(void *userdata, struct fuse_conn_info *conn)
6e7d01
 {
6e7d01
     struct lo_data *lo = (struct lo_data *)userdata;
6e7d01
@@ -788,9 +820,9 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr,
6e7d01
         if (fi) {
6e7d01
             truncfd = fd;
6e7d01
         } else {
6e7d01
-            sprintf(procname, "%i", ifd);
6e7d01
-            truncfd = openat(lo->proc_self_fd, procname, O_RDWR);
6e7d01
+            truncfd = lo_inode_open(lo, inode, O_RDWR);
6e7d01
             if (truncfd < 0) {
6e7d01
+                errno = -truncfd;
6e7d01
                 goto out_err;
6e7d01
             }
6e7d01
         }
6e7d01
@@ -894,7 +926,7 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
6e7d01
     struct lo_inode *dir = lo_inode(req, parent);
6e7d01
 
6e7d01
     if (inodep) {
6e7d01
-        *inodep = NULL;
6e7d01
+        *inodep = NULL; /* in case there is an error */
6e7d01
     }
6e7d01
 
6e7d01
     /*
6e7d01
@@ -1725,19 +1757,26 @@ static void update_open_flags(int writeback, struct fuse_file_info *fi)
6e7d01
     fi->flags &= ~O_DIRECT;
6e7d01
 }
6e7d01
 
6e7d01
+/*
6e7d01
+ * Open a regular file, set up an fd mapping, and fill out the struct
6e7d01
+ * fuse_file_info for it. If existing_fd is not negative, use that fd instead
6e7d01
+ * opening a new one. Takes ownership of existing_fd.
6e7d01
+ *
6e7d01
+ * Returns 0 on success or a positive errno.
6e7d01
+ */
6e7d01
 static int lo_do_open(struct lo_data *lo, struct lo_inode *inode,
6e7d01
-                      struct fuse_file_info *fi)
6e7d01
+                      int existing_fd, struct fuse_file_info *fi)
6e7d01
 {
6e7d01
-    char buf[64];
6e7d01
     ssize_t fh;
6e7d01
-    int fd;
6e7d01
+    int fd = existing_fd;
6e7d01
 
6e7d01
     update_open_flags(lo->writeback, fi);
6e7d01
 
6e7d01
-    sprintf(buf, "%i", inode->fd);
6e7d01
-    fd = openat(lo->proc_self_fd, buf, fi->flags & ~O_NOFOLLOW);
6e7d01
-    if (fd == -1) {
6e7d01
-        return errno;
6e7d01
+    if (fd < 0) {
6e7d01
+        fd = lo_inode_open(lo, inode, fi->flags);
6e7d01
+        if (fd < 0) {
6e7d01
+            return -fd;
6e7d01
+        }
6e7d01
     }
6e7d01
 
6e7d01
     pthread_mutex_lock(&lo->mutex);
6e7d01
@@ -1760,9 +1799,10 @@ static int lo_do_open(struct lo_data *lo, struct lo_inode *inode,
6e7d01
 static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name,
6e7d01
                       mode_t mode, struct fuse_file_info *fi)
6e7d01
 {
6e7d01
-    int fd;
6e7d01
+    int fd = -1;
6e7d01
     struct lo_data *lo = lo_data(req);
6e7d01
     struct lo_inode *parent_inode;
6e7d01
+    struct lo_inode *inode = NULL;
6e7d01
     struct fuse_entry_param e;
6e7d01
     int err;
6e7d01
     struct lo_cred old = {};
6e7d01
@@ -1788,36 +1828,38 @@ static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name,
6e7d01
 
6e7d01
     update_open_flags(lo->writeback, fi);
6e7d01
 
6e7d01
-    fd = openat(parent_inode->fd, name, (fi->flags | O_CREAT) & ~O_NOFOLLOW,
6e7d01
-                mode);
6e7d01
+    /* Try to create a new file but don't open existing files */
6e7d01
+    fd = openat(parent_inode->fd, name, fi->flags | O_CREAT | O_EXCL, mode);
6e7d01
     err = fd == -1 ? errno : 0;
6e7d01
-    lo_restore_cred(&old;;
6e7d01
 
6e7d01
-    if (!err) {
6e7d01
-        ssize_t fh;
6e7d01
+    lo_restore_cred(&old;;
6e7d01
 
6e7d01
-        pthread_mutex_lock(&lo->mutex);
6e7d01
-        fh = lo_add_fd_mapping(lo, fd);
6e7d01
-        pthread_mutex_unlock(&lo->mutex);
6e7d01
-        if (fh == -1) {
6e7d01
-            close(fd);
6e7d01
-            err = ENOMEM;
6e7d01
-            goto out;
6e7d01
-        }
6e7d01
+    /* Ignore the error if file exists and O_EXCL was not given */
6e7d01
+    if (err && (err != EEXIST || (fi->flags & O_EXCL))) {
6e7d01
+        goto out;
6e7d01
+    }
6e7d01
 
6e7d01
-        fi->fh = fh;
6e7d01
-        err = lo_do_lookup(req, parent, name, &e, NULL);
6e7d01
+    err = lo_do_lookup(req, parent, name, &e, &inode;;
6e7d01
+    if (err) {
6e7d01
+        goto out;
6e7d01
     }
6e7d01
-    if (lo->cache == CACHE_NONE) {
6e7d01
-        fi->direct_io = 1;
6e7d01
-    } else if (lo->cache == CACHE_ALWAYS) {
6e7d01
-        fi->keep_cache = 1;
6e7d01
+
6e7d01
+    err = lo_do_open(lo, inode, fd, fi);
6e7d01
+    fd = -1; /* lo_do_open() takes ownership of fd */
6e7d01
+    if (err) {
6e7d01
+        /* Undo lo_do_lookup() nlookup ref */
6e7d01
+        unref_inode_lolocked(lo, inode, 1);
6e7d01
     }
6e7d01
 
6e7d01
 out:
6e7d01
+    lo_inode_put(lo, &inode;;
6e7d01
     lo_inode_put(lo, &parent_inode);
6e7d01
 
6e7d01
     if (err) {
6e7d01
+        if (fd >= 0) {
6e7d01
+            close(fd);
6e7d01
+        }
6e7d01
+
6e7d01
         fuse_reply_err(req, err);
6e7d01
     } else {
6e7d01
         fuse_reply_create(req, &e, fi);
6e7d01
@@ -1831,7 +1873,6 @@ static struct lo_inode_plock *lookup_create_plock_ctx(struct lo_data *lo,
6e7d01
                                                       pid_t pid, int *err)
6e7d01
 {
6e7d01
     struct lo_inode_plock *plock;
6e7d01
-    char procname[64];
6e7d01
     int fd;
6e7d01
 
6e7d01
     plock =
6e7d01
@@ -1848,12 +1889,10 @@ static struct lo_inode_plock *lookup_create_plock_ctx(struct lo_data *lo,
6e7d01
     }
6e7d01
 
6e7d01
     /* Open another instance of file which can be used for ofd locks. */
6e7d01
-    sprintf(procname, "%i", inode->fd);
6e7d01
-
6e7d01
     /* TODO: What if file is not writable? */
6e7d01
-    fd = openat(lo->proc_self_fd, procname, O_RDWR);
6e7d01
-    if (fd == -1) {
6e7d01
-        *err = errno;
6e7d01
+    fd = lo_inode_open(lo, inode, O_RDWR);
6e7d01
+    if (fd < 0) {
6e7d01
+        *err = -fd;
6e7d01
         free(plock);
6e7d01
         return NULL;
6e7d01
     }
6e7d01
@@ -2000,7 +2039,7 @@ static void lo_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi)
6e7d01
         return;
6e7d01
     }
6e7d01
 
6e7d01
-    err = lo_do_open(lo, inode, fi);
6e7d01
+    err = lo_do_open(lo, inode, -1, fi);
6e7d01
     lo_inode_put(lo, &inode;;
6e7d01
     if (err) {
6e7d01
         fuse_reply_err(req, err);
6e7d01
@@ -2056,39 +2095,40 @@ static void lo_flush(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi)
6e7d01
 static void lo_fsync(fuse_req_t req, fuse_ino_t ino, int datasync,
6e7d01
                      struct fuse_file_info *fi)
6e7d01
 {
6e7d01
+    struct lo_inode *inode = lo_inode(req, ino);
6e7d01
+    struct lo_data *lo = lo_data(req);
6e7d01
     int res;
6e7d01
     int fd;
6e7d01
-    char *buf;
6e7d01
 
6e7d01
     fuse_log(FUSE_LOG_DEBUG, "lo_fsync(ino=%" PRIu64 ", fi=0x%p)\n", ino,
6e7d01
              (void *)fi);
6e7d01
 
6e7d01
-    if (!fi) {
6e7d01
-        struct lo_data *lo = lo_data(req);
6e7d01
-
6e7d01
-        res = asprintf(&buf, "%i", lo_fd(req, ino));
6e7d01
-        if (res == -1) {
6e7d01
-            return (void)fuse_reply_err(req, errno);
6e7d01
-        }
6e7d01
+    if (!inode) {
6e7d01
+        fuse_reply_err(req, EBADF);
6e7d01
+        return;
6e7d01
+    }
6e7d01
 
6e7d01
-        fd = openat(lo->proc_self_fd, buf, O_RDWR);
6e7d01
-        free(buf);
6e7d01
-        if (fd == -1) {
6e7d01
-            return (void)fuse_reply_err(req, errno);
6e7d01
+    if (!fi) {
6e7d01
+        fd = lo_inode_open(lo, inode, O_RDWR);
6e7d01
+        if (fd < 0) {
6e7d01
+            res = -fd;
6e7d01
+            goto out;
6e7d01
         }
6e7d01
     } else {
6e7d01
         fd = lo_fi_fd(req, fi);
6e7d01
     }
6e7d01
 
6e7d01
     if (datasync) {
6e7d01
-        res = fdatasync(fd);
6e7d01
+        res = fdatasync(fd) == -1 ? errno : 0;
6e7d01
     } else {
6e7d01
-        res = fsync(fd);
6e7d01
+        res = fsync(fd) == -1 ? errno : 0;
6e7d01
     }
6e7d01
     if (!fi) {
6e7d01
         close(fd);
6e7d01
     }
6e7d01
-    fuse_reply_err(req, res == -1 ? errno : 0);
6e7d01
+out:
6e7d01
+    lo_inode_put(lo, &inode;;
6e7d01
+    fuse_reply_err(req, res);
6e7d01
 }
6e7d01
 
6e7d01
 static void lo_read(fuse_req_t req, fuse_ino_t ino, size_t size, off_t offset,
6e7d01
-- 
6e7d01
2.18.2
6e7d01