Patchwork [2/2] virtio-9p: Use chroot interface in passthrough model

login
register
mail settings
Submitter Mohan Kumar M
Date Nov. 15, 2010, 2:53 p.m.
Message ID <1289832810-4474-1-git-send-email-mohan@in.ibm.com>
Download mbox | patch
Permalink /patch/71234/
State New
Headers show

Comments

Mohan Kumar M - Nov. 15, 2010, 2:53 p.m.
Make use of chroot interfaces for passthrough security model to fix the
vulnerability in following symbolic links.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 hw/virtio-9p-local.c |  284 ++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 218 insertions(+), 66 deletions(-)

Patch

diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index 656bfb3..4b72dec 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -19,16 +19,91 @@ 
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <attr/xattr.h>
+#include <libgen.h>
+
+static int get_fd(FsContext *fs_ctx, const char *path, int flags, FsCred *credp)
+{
+    V9fsOpenRequest request;
+    int fd, error = 0;
+
+    memset(&request, 0, sizeof(request));
+    request.data.path_len = strlen(path);
+    request.path.path = qemu_strdup(path);
+    request.data.flags = flags;
+    if (credp) {
+        request.data.mode = credp->fc_mode;
+        request.data.uid = credp->fc_uid;
+        request.data.gid = credp->fc_gid;
+        request.data.dev = credp->fc_rdev;
+    }
+    fd = v9fs_getfd(&request, &error, fs_ctx);
+    if (error) {
+        errno = error;
+    } else {
+        errno = error;
+    }
+    qemu_strdup(request.path.path);
+    return fd;
+}
+
+static int get_pfd(FsContext *fs_ctx, const char *path)
+{
+    V9fsOpenRequest request;
+    int fd, error = 0;
+    char *dpath = qemu_strdup(path);
+
+    memset(&request, 0, sizeof(request));
+    request.path.path = dirname(dpath);
+    request.data.path_len = strlen(request.path.path);
+    request.data.flags = O_RDONLY | O_DIRECTORY | O_NOFOLLOW;
+    fd = v9fs_getfd(&request, &error, fs_ctx);
+    if (error) {
+        errno = error;
+    } else {
+        errno = 0;
+    }
+    qemu_free(dpath);
+    return fd;
+}
 
+static int do_symlink(FsContext *fs_ctx, const char *oldpath,
+                        const char *newpath, FsCred *credp)
+{
+    V9fsOpenRequest request;
+    int fd, error = 0;
+
+    memset(&request, 0, sizeof(request));
+    request.data.path_len = strlen(newpath);
+    request.path.path = qemu_strdup(newpath);
+    request.data.oldpath_len = strlen(oldpath);
+    request.path.old_path = qemu_strdup(oldpath);
+    request.data.flags = S_IFLNK | O_CREAT;
+
+    if (credp) {
+        request.data.mode = credp->fc_mode;
+        request.data.uid = credp->fc_uid;
+        request.data.gid = credp->fc_gid;
+        request.data.dev = credp->fc_rdev;
+    }
+    fd = v9fs_getfd(&request, &error, fs_ctx);
+    if (error) {
+        errno = error;
+    } else {
+        errno = error;
+    }
+    qemu_strdup(request.path.path);
+    return fd;
+}
 
 static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
 {
     int err;
-    err =  lstat(rpath(fs_ctx, path), stbuf);
-    if (err) {
-        return err;
-    }
+
     if (fs_ctx->fs_sm == SM_MAPPED) {
+        err =  lstat(rpath(fs_ctx, path), stbuf);
+        if (err) {
+            return err;
+        }
         /* Actual credentials are part of extended attrs */
         uid_t tmp_uid;
         gid_t tmp_gid;
@@ -50,6 +125,22 @@  static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
                         sizeof(dev_t)) > 0) {
                 stbuf->st_rdev = tmp_dev;
         }
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        int pfd;
+        char *base, *basep;
+
+        base = qemu_strdup(path);
+        basep = basename(base);
+
+        pfd = get_pfd(fs_ctx, path);
+        err = fstatat(pfd, basep, stbuf, AT_SYMLINK_NOFOLLOW);
+        close(pfd);
+        free(base);
+    } else {
+        err =  lstat(rpath(fs_ctx, path), stbuf);
+        if (err) {
+            return err;
+        }
     }
     return err;
 }
@@ -88,21 +179,13 @@  static int local_set_xattr(const char *path, FsCred *credp)
     return 0;
 }
 
-static int local_post_create_passthrough(FsContext *fs_ctx, const char *path,
-        FsCred *credp)
+static int local_post_create_none(FsContext *fs_ctx, const char *path,
+                FsCred *credp)
 {
     if (chmod(rpath(fs_ctx, path), credp->fc_mode & 07777) < 0) {
         return -1;
     }
-    if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid) < 0) {
-        /*
-         * If we fail to change ownership and if we are
-         * using security model none. Ignore the error
-         */
-        if (fs_ctx->fs_sm != SM_NONE) {
-            return -1;
-        }
-    }
+    lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
     return 0;
 }
 
@@ -121,9 +204,16 @@  static ssize_t local_readlink(FsContext *fs_ctx, const char *path,
         } while (tsize == -1 && errno == EINTR);
         close(fd);
         return tsize;
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         tsize = readlink(rpath(fs_ctx, path), buf, bufsz);
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        int pfd;
+        char *base;
+        base = qemu_strdup(path);
+        pfd = get_pfd(fs_ctx, path);
+        tsize = readlinkat(pfd, basename(base), buf, bufsz);
+        qemu_free(base);
+        close(pfd);
     }
     return tsize;
 }
@@ -140,7 +230,11 @@  static int local_closedir(FsContext *ctx, DIR *dir)
 
 static int local_open(FsContext *ctx, const char *path, int flags)
 {
-    return open(rpath(ctx, path), flags);
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        return get_fd(ctx, path, flags, 0);
+    } else {
+        return open(rpath(ctx, path), flags);
+    }
 }
 
 static DIR *local_opendir(FsContext *ctx, const char *path)
@@ -225,17 +319,25 @@  static int local_mknod(FsContext *fs_ctx, const char *path, FsCred *credp)
             serrno = errno;
             goto err_end;
         }
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         err = mknod(rpath(fs_ctx, path), credp->fc_mode, credp->fc_rdev);
         if (err == -1) {
             return err;
         }
-        err = local_post_create_passthrough(fs_ctx, path, credp);
+        err = local_post_create_none(fs_ctx, path, credp);
         if (err == -1) {
             serrno = errno;
             goto err_end;
         }
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        int fd;
+        fd = get_fd(fs_ctx, path, O_CREAT | S_IFCHR, credp);
+        if (fd < 0) {
+            err = fd;
+            serrno = errno;
+            goto err_end;
+        }
+        err = 0;
     }
     return err;
 
@@ -262,17 +364,25 @@  static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred *credp)
             serrno = errno;
             goto err_end;
         }
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         err = mkdir(rpath(fs_ctx, path), credp->fc_mode);
         if (err == -1) {
             return err;
         }
-        err = local_post_create_passthrough(fs_ctx, path, credp);
+        err = local_post_create_none(fs_ctx, path, credp);
         if (err == -1) {
             serrno = errno;
             goto err_end;
         }
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        int fd;
+        fd = get_fd(fs_ctx, path, O_CREAT | S_IFDIR, credp);
+        if (fd < 0) {
+            err = fd;
+            serrno = errno;
+            goto err_end;
+        }
+        err = 0;
     }
     return err;
 
@@ -332,17 +442,18 @@  static int local_open2(FsContext *fs_ctx, const char *path, int flags,
             serrno = errno;
             goto err_end;
         }
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         fd = open(rpath(fs_ctx, path), flags, credp->fc_mode);
         if (fd == -1) {
             return fd;
         }
-        err = local_post_create_passthrough(fs_ctx, path, credp);
+        err = local_post_create_none(fs_ctx, path, credp);
         if (err == -1) {
             serrno = errno;
             goto err_end;
         }
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        fd = get_fd(fs_ctx, path, flags, credp);
     }
     return fd;
 
@@ -389,23 +500,17 @@  static int local_symlink(FsContext *fs_ctx, const char *oldpath,
             serrno = errno;
             goto err_end;
         }
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         err = symlink(oldpath, rpath(fs_ctx, newpath));
         if (err) {
             return err;
         }
-        err = lchown(rpath(fs_ctx, newpath), credp->fc_uid, credp->fc_gid);
+        lchown(rpath(fs_ctx, newpath), credp->fc_uid, credp->fc_gid);
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        err = do_symlink(fs_ctx, oldpath, newpath, credp);
         if (err == -1) {
-            /*
-             * If we fail to change ownership and if we are
-             * using security model none. Ignore the error
-             */
-            if (fs_ctx->fs_sm != SM_NONE) {
-                serrno = errno;
-                goto err_end;
-            } else
-                err = 0;
+            serrno = errno;
+            goto err_end;
         }
     }
     return err;
@@ -418,22 +523,33 @@  err_end:
 
 static int local_link(FsContext *ctx, const char *oldpath, const char *newpath)
 {
-    char *tmp = qemu_strdup(rpath(ctx, oldpath));
     int err, serrno = 0;
 
-    if (tmp == NULL) {
-        return -ENOMEM;
-    }
-
-    err = link(tmp, rpath(ctx, newpath));
-    if (err == -1) {
-        serrno = errno;
-    }
-
-    qemu_free(tmp);
-
-    if (err == -1) {
-        errno = serrno;
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        int opfd, npfd;
+        char *obase, *nbase;
+        obase = qemu_strdup(oldpath);
+        nbase = qemu_strdup(newpath);
+        opfd = get_pfd(ctx, obase);
+        npfd = get_pfd(ctx, nbase);
+        err = linkat(opfd, basename(obase), npfd, basename(nbase), 0);
+        if (err == -1) {
+            serrno = errno;
+        }
+        qemu_free(obase);
+        qemu_free(nbase);
+        close(opfd);
+        close(npfd);
+    } else {
+        char *tmp = qemu_strdup(rpath(ctx, oldpath));
+        if (tmp == NULL) {
+            return -ENOMEM;
+        }
+        err = link(tmp, rpath(ctx, newpath));
+        if (err == -1) {
+            serrno = errno;
+        }
+        qemu_free(tmp);
     }
 
     return err;
@@ -441,24 +557,49 @@  static int local_link(FsContext *ctx, const char *oldpath, const char *newpath)
 
 static int local_truncate(FsContext *ctx, const char *path, off_t size)
 {
-    return truncate(rpath(ctx, path), size);
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        int fd, retval;
+        fd = get_fd(ctx, path, O_RDWR, 0);
+        retval = ftruncate(fd, size);
+        close(fd);
+        return retval;
+    } else {
+        return truncate(rpath(ctx, path), size);
+    }
 }
 
 static int local_rename(FsContext *ctx, const char *oldpath,
                         const char *newpath)
 {
-    char *tmp;
-    int err;
-
-    tmp = qemu_strdup(rpath(ctx, oldpath));
+    int err, serrno = 0;
 
-    err = rename(tmp, rpath(ctx, newpath));
-    if (err == -1) {
-        int serrno = errno;
-        qemu_free(tmp);
-        errno = serrno;
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        int opfd, npfd;
+        char *obase, *nbase;
+        obase = qemu_strdup(oldpath);
+        nbase = qemu_strdup(newpath);
+        opfd = get_pfd(ctx, obase);
+        npfd = get_pfd(ctx, nbase);
+        err = renameat(opfd, basename(obase), npfd, basename(nbase));
+        if (err == -1) {
+            serrno = errno;
+        }
+        qemu_free(obase);
+        qemu_free(nbase);
+        close(opfd);
+        close(npfd);
     } else {
-        qemu_free(tmp);
+        char *tmp;
+        tmp = qemu_strdup(rpath(ctx, oldpath));
+
+        err = rename(tmp, rpath(ctx, newpath));
+        if (err == -1) {
+            int serrno = errno;
+            qemu_free(tmp);
+            errno = serrno;
+        } else {
+            qemu_free(tmp);
+        }
     }
 
     return err;
@@ -480,9 +621,20 @@  static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp)
 }
 
 static int local_utimensat(FsContext *s, const char *path,
-		       const struct timespec *buf)
-{
-    return utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+                const struct timespec *buf)
+{
+    if (s->fs_sm == SM_PASSTHROUGH) {
+        int pfd, retval;
+        char *base;
+        base = qemu_strdup(path);
+        pfd = get_pfd(s, path);
+        retval = utimensat(pfd, basename(base), buf,
+                        AT_SYMLINK_NOFOLLOW);
+        close(pfd);
+        return retval;
+    } else {
+        return utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+    }
 }
 
 static int local_remove(FsContext *ctx, const char *path)