Patchwork [V9,13/13] virtio-9p: Chroot environment for other functions

login
register
mail settings
Submitter Mohan Kumar M
Date March 18, 2011, 3:28 a.m.
Message ID <1300418881-20972-14-git-send-email-mohan@in.ibm.com>
Download mbox | patch
Permalink /patch/87451/
State New
Headers show

Comments

Mohan Kumar M - March 18, 2011, 3:28 a.m.
Add chroot functionality for systemcalls that can operate on a file
using relative directory file descriptor.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 hw/9pfs/virtio-9p-local.c |  114 +++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 104 insertions(+), 10 deletions(-)
Stefan Hajnoczi - March 28, 2011, 9:06 p.m.
On Fri, Mar 18, 2011 at 08:58:01AM +0530, M. Mohan Kumar wrote:
> +static int get_dirfd(FsContext *fs_ctx, const char *path)
> +{
> +    int fd;
> +    char *dpath = qemu_strdup(path);
> +    char *last_component;
> +
> +    /* path can not contain ".." */
> +    last_component = strrchr(path, '/');
> +    if (last_component && !strcmp(last_component, "/..")) {
> +        error_report("9p path request contains \"..\": %s\n", path);
> +        qemu_free(dpath);
> +        errno = EFAULT;
> +        return -1;
> +    }
> +    fd = passthrough_request(fs_ctx, NULL, dirname(dpath), 0, NULL, T_OPEN);
> +    if (fd < 0) {
> +        qemu_free(dpath);
> +        errno = -fd;
> +        fd = -1;
> +    }
> +    return fd;
> +}

dpath is leaked in the success case.  I suggest writing the function like this:
static int get_dirfd(FsContext *fs_ctx, const char *path)
{
    int fd;
    char *dpath;
    char *last_component;

    /* path can not contain ".." */
    last_component = strrchr(path, '/');
    if (last_component && !strcmp(last_component, "/..")) {
        error_report("9p path request contains \"..\": %s\n", path);
        errno = EFAULT;
        return -1;
    }
    dpath = qemu_strdup(path);
    fd = passthrough_request(fs_ctx, NULL, dirname(dpath), 0, NULL, T_OPEN);
    qemu_free(dpath);
    if (fd < 0) {
        errno = -fd;
        fd = -1;
    }
    return fd;
}

> -static int local_utimensat(FsContext *s, const char *path,
> -                           const struct timespec *buf)
> +static int local_utimensat(FsContext *fs_ctx, const char *path,
> +                const struct timespec *buf)
>  {
> -    return qemu_utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
> +    if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
> +        int fd, retval;
> +        fd = passthrough_request(fs_ctx, NULL, path,
> +                        O_RDONLY | O_NONBLOCK | O_NOFOLLOW, NULL, T_OPEN);
> +        if (fd < 0) {
> +            errno = -fd;
> +            return -1;
> +        }
> +        retval = futimens(fd, buf);
> +        close(fd);
> +        return retval;

errno is clobbered here.

Stefan
Mohan Kumar M - March 29, 2011, 5:55 a.m.
Thanks Stefan. I will send next version with incorporating your suggestions.
 
> dpath is leaked in the success case.  I suggest writing the function like
> this: static int get_dirfd(FsContext *fs_ctx, const char *path)
> {
>     int fd;
>     char *dpath;
>     char *last_component;
> 
>     /* path can not contain ".." */
>     last_component = strrchr(path, '/');
>     if (last_component && !strcmp(last_component, "/..")) {
>         error_report("9p path request contains \"..\": %s\n", path);
>         errno = EFAULT;
>         return -1;
>     }
>     dpath = qemu_strdup(path);
>     fd = passthrough_request(fs_ctx, NULL, dirname(dpath), 0, NULL,
> T_OPEN); qemu_free(dpath);
>     if (fd < 0) {
>         errno = -fd;
>         fd = -1;
>     }
>     return fd;
> }

Ok, I will rewrite the function as you mentioned.

> 
> > -static int local_utimensat(FsContext *s, const char *path,
> > -                           const struct timespec *buf)
> > +static int local_utimensat(FsContext *fs_ctx, const char *path,
> > +                const struct timespec *buf)
> > 
> >  {
> > 
> > -    return qemu_utimensat(AT_FDCWD, rpath(s, path), buf,
> > AT_SYMLINK_NOFOLLOW); +    if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
> > +        int fd, retval;
> > +        fd = passthrough_request(fs_ctx, NULL, path,
> > +                        O_RDONLY | O_NONBLOCK | O_NOFOLLOW, NULL,
> > T_OPEN); +        if (fd < 0) {
> > +            errno = -fd;
> > +            return -1;
> > +        }
> > +        retval = futimens(fd, buf);
> > +        close(fd);
> > +        return retval;
> 
> errno is clobbered here.

I will fix it.

----
M. Mohan Kumar

Patch

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index b2e4c51..7c04075 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -22,6 +22,8 @@ 
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <attr/xattr.h>
+#include <libgen.h>
+#include "qemu-error.h"
 
 /* Helper routine to fill V9fsFileObjectRequest structure */
 static int fill_fileobjectrequest(V9fsFileObjectRequest *request,
@@ -80,14 +82,42 @@  static int passthrough_request(FsContext *fs_ctx, const char *old_path,
     return retval;
 }
 
+/*
+ * Returns file descriptor of dirname(path)
+ * This fd can be used by *at functions
+ */
+static int get_dirfd(FsContext *fs_ctx, const char *path)
+{
+    int fd;
+    char *dpath = qemu_strdup(path);
+    char *last_component;
+
+    /* path can not contain ".." */
+    last_component = strrchr(path, '/');
+    if (last_component && !strcmp(last_component, "/..")) {
+        error_report("9p path request contains \"..\": %s\n", path);
+        qemu_free(dpath);
+        errno = EFAULT;
+        return -1;
+    }
+    fd = passthrough_request(fs_ctx, NULL, dirname(dpath), 0, NULL, T_OPEN);
+    if (fd < 0) {
+        qemu_free(dpath);
+        errno = -fd;
+        fd = -1;
+    }
+    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;
@@ -109,6 +139,27 @@  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, serrno = 0;
+        char *tmp_path;
+
+        pfd = get_dirfd(fs_ctx, path);
+        if (pfd < 0) {
+            return -1;
+        }
+        tmp_path = qemu_strdup(path);
+        err = fstatat(pfd, basename(tmp_path), stbuf, AT_SYMLINK_NOFOLLOW);
+        if (err < 0) {
+            serrno = errno;
+        }
+        close(pfd);
+        qemu_free(tmp_path);
+        errno = serrno;
+    } else {
+        err =  lstat(rpath(fs_ctx, path), stbuf);
+        if (err) {
+            return err;
+        }
     }
     return err;
 }
@@ -173,9 +224,24 @@  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, serrno = 0;
+        char *tmp_path;
+        pfd = get_dirfd(fs_ctx, path);
+        if (pfd < 0) {
+            return -1;
+        }
+
+        tmp_path = qemu_strdup(path);
+        tsize = readlinkat(pfd, basename(tmp_path), buf, bufsz);
+        if (tsize < 0) {
+            serrno = errno;
+        }
+        close(pfd);
+        qemu_free(tmp_path);
+        errno = serrno;
     }
     return tsize;
 }
@@ -514,7 +580,21 @@  static int local_link(FsContext *fs_ctx, const char *oldpath,
 
 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, serrno;
+        fd = passthrough_request(ctx, NULL, path, O_RDWR, NULL, T_OPEN);
+        if (fd < 0) {
+            errno = -fd;
+            return -1;
+        }
+        retval = ftruncate(fd, size);
+        serrno = errno;
+        close(fd);
+        errno = serrno;
+        return retval;
+    } else {
+        return truncate(rpath(ctx, path), size);
+    }
 }
 
 static int local_rename(FsContext *ctx, const char *oldpath,
@@ -557,10 +637,24 @@  static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp)
     return -1;
 }
 
-static int local_utimensat(FsContext *s, const char *path,
-                           const struct timespec *buf)
+static int local_utimensat(FsContext *fs_ctx, const char *path,
+                const struct timespec *buf)
 {
-    return qemu_utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+    if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        int fd, retval;
+        fd = passthrough_request(fs_ctx, NULL, path,
+                        O_RDONLY | O_NONBLOCK | O_NOFOLLOW, NULL, T_OPEN);
+        if (fd < 0) {
+            errno = -fd;
+            return -1;
+        }
+        retval = futimens(fd, buf);
+        close(fd);
+        return retval;
+    } else {
+        return utimensat(AT_FDCWD, rpath(fs_ctx, path), buf,
+                        AT_SYMLINK_NOFOLLOW);
+    }
 }
 
 static int local_remove(FsContext *ctx, const char *path)