diff mbox

[-V3,5/7] virtio-9p: Implemented security model for symlink and link.

Message ID 1274477170-7658-6-git-send-email-jvrao@linux.vnet.ibm.com
State New
Headers show

Commit Message

jvrao May 21, 2010, 9:26 p.m. UTC
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
 hw/file-op-9p.h      |    4 +-
 hw/virtio-9p-local.c |   87 +++++++++++++++++++++++++++++++++++++++----------
 hw/virtio-9p.c       |   24 ++++++++++----
 3 files changed, 88 insertions(+), 27 deletions(-)

Comments

Anthony Liguori May 24, 2010, 8:13 p.m. UTC | #1
On 05/21/2010 04:26 PM, Venkateswararao Jujjuri (JV) wrote:
> Signed-off-by: Venkateswararao Jujjuri<jvrao@linux.vnet.ibm.com>
> ---
>   hw/file-op-9p.h      |    4 +-
>   hw/virtio-9p-local.c |   87 +++++++++++++++++++++++++++++++++++++++----------
>   hw/virtio-9p.c       |   24 ++++++++++----
>   3 files changed, 88 insertions(+), 27 deletions(-)
>
> diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
> index 752fbad..0a9c10a 100644
> --- a/hw/file-op-9p.h
> +++ b/hw/file-op-9p.h
> @@ -54,8 +54,8 @@ typedef struct FileOperations
>       int (*mknod)(FsContext *, const char *, FsCred *);
>       int (*utime)(FsContext *, const char *, const struct utimbuf *);
>       int (*remove)(FsContext *, const char *);
> -    int (*symlink)(FsContext *, const char *, const char *);
> -    int (*link)(FsContext *, const char *, const char *);
> +    int (*symlink)(FsContext *, const char *, const char *, FsCred *);
> +    int (*link)(FsContext *, const char *, const char *, FsCred *);
>       int (*setuid)(FsContext *, uid_t);
>       int (*close)(FsContext *, int);
>       int (*closedir)(FsContext *, DIR *);
> diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
> index bfcd695..ca0f065 100644
> --- a/hw/virtio-9p-local.c
> +++ b/hw/virtio-9p-local.c
> @@ -64,12 +64,25 @@ static int local_set_xattr(const char *path, FsCred *credp)
>           }
>       }
>        return 0;
> - }
> +}
>
> -static ssize_t local_readlink(FsContext *ctx, const char *path,
> -                                char *buf, size_t bufsz)
> +static ssize_t local_readlink(FsContext *fs_ctx, const char *path,
> +        char *buf, size_t bufsz)
>   {
> -    return readlink(rpath(ctx, path), buf, bufsz);
> +    ssize_t tsize = -1;
> +    if (fs_ctx->fs_sm == sm_mapped) {
> +        int fd;
> +        fd = open(rpath(fs_ctx, path), O_RDONLY);
> +        if (fd == -1) {
> +            return -1;
> +        }
> +        tsize = read(fd, (void *)buf, bufsz);
> +        close(fd);
> +        return tsize;
> +    } else if (fs_ctx->fs_sm == sm_passthrough) {
> +        tsize = readlink(rpath(fs_ctx, path), buf, bufsz);
> +    }
> +    return tsize;
>   }
>
>   static int local_close(FsContext *ctx, int fd)
> @@ -247,32 +260,70 @@ err_end:
>       return err;
>   }
>
> -static int local_symlink(FsContext *ctx, const char *oldpath,
> -                            const char *newpath)
> +
> +static int local_symlink(FsContext *fs_ctx, const char *oldpath,
> +        const char *newpath, FsCred *credp)
>   {
> -    return symlink(oldpath, rpath(ctx, newpath));
> +    int err = -1;
> +    /* Determine the security model */
> +    if (fs_ctx->fs_sm == sm_mapped) {
> +        int fd;
> +        ssize_t oldpath_size, write_size;
> +        fd = open(rpath(fs_ctx, newpath), O_CREAT|O_EXCL|O_RDWR,
> +                SM_LOCAL_MODE_BITS);
> +        if (fd == -1) {
> +            return fd;
> +        }
> +        /* Write the oldpath (target) to the file. */
> +        oldpath_size = strlen(oldpath) + 1;
> +        write_size = write(fd, (void *)oldpath, oldpath_size);
>    

Signals are pretty frequent in qemu so not handling EINTR is asking for 
trouble.

Regards,

Anthony Liguori

> +        if (write_size != oldpath_size) {
> +            close(fd);
> +            err = -1;
> +            goto err_end;
> +        }
> +        close(fd);
> +        /* Set cleint credentials in symlink's xattr */
> +        credp->fc_mode = credp->fc_mode|S_IFLNK;
> +        err = local_set_xattr(rpath(fs_ctx, newpath), credp);
> +        if (err == -1) {
> +            goto err_end;
> +        }
> +    } else if (fs_ctx->fs_sm == sm_passthrough) {
> +        err = symlink(oldpath, rpath(fs_ctx, newpath));
> +        if (err) {
> +            return err;
> +        }
> +        err = chmod(rpath(fs_ctx, newpath), credp->fc_mode&  07777);
> +        if (err == -1) {
> +            goto err_end;
> +        }
> +        err = chown(rpath(fs_ctx, newpath), credp->fc_uid, credp->fc_gid);
> +        if (err == -1) {
> +            goto err_end;
> +        }
> +    }
> +    return err;
> +
> +err_end:
> +    remove(rpath(fs_ctx, newpath));
> +    return err;
>   }
>
> -static int local_link(FsContext *ctx, const char *oldpath, const char *newpath)
> +static int local_link(FsContext *fs_ctx, const char *oldpath,
> +        const char *newpath, FsCred *credp)
>   {
> -    char *tmp = qemu_strdup(rpath(ctx, oldpath));
> -    int err, serrno = 0;
> +    char *tmp = qemu_strdup(rpath(fs_ctx, oldpath));
> +    int err;
>
>       if (tmp == NULL) {
>           return -ENOMEM;
>       }
>
> -    err = link(tmp, rpath(ctx, newpath));
> -    if (err == -1) {
> -        serrno = errno;
> -    }
> +    err = link(tmp, rpath(fs_ctx, newpath));
>
>       qemu_free(tmp);
>
> -    if (err == -1) {
> -        errno = serrno;
> -    }
> -
>       return err;
>   }
>
> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> index 30f649d..fbc846b 100644
> --- a/hw/virtio-9p.c
> +++ b/hw/virtio-9p.c
> @@ -197,15 +197,25 @@ static int v9fs_do_open2(V9fsState *s, V9fsCreateState *vs)
>       return s->ops->open2(&s->ctx, vs->fullname.data, flags,&cred);
>   }
>
> -static int v9fs_do_symlink(V9fsState *s, V9fsString *oldpath,
> -                            V9fsString *newpath)
> +static int v9fs_do_symlink(V9fsState *s, V9fsCreateState *vs)
>   {
> -    return s->ops->symlink(&s->ctx, oldpath->data, newpath->data);
> +    FsCred cred;
> +    cred_init(&cred);
> +    cred.fc_uid = vs->fidp->uid;
> +    cred.fc_mode = vs->perm | 0777;
> +
> +    return s->ops->symlink(&s->ctx, vs->extension.data, vs->fullname.data,
> +&cred);
>   }
>
> -static int v9fs_do_link(V9fsState *s, V9fsString *oldpath, V9fsString *newpath)
> +static int v9fs_do_link(V9fsState *s, V9fsFidState *nfidp, V9fsCreateState *vs)
>   {
> -    return s->ops->link(&s->ctx, oldpath->data, newpath->data);
> +    FsCred cred;
> +    cred_init(&cred);
> +    cred.fc_uid = nfidp->uid;
> +    cred.fc_mode = vs->perm&  0777;
> +
> +    return s->ops->link(&s->ctx, nfidp->path.data, vs->fullname.data,&cred);
>   }
>
>   static int v9fs_do_truncate(V9fsState *s, V9fsString *path, off_t size)
> @@ -1782,7 +1792,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
>           err = v9fs_do_mkdir(s, vs);
>           v9fs_create_post_mkdir(s, vs, err);
>       } else if (vs->perm&  P9_STAT_MODE_SYMLINK) {
> -        err = v9fs_do_symlink(s,&vs->extension,&vs->fullname);
> +        err = v9fs_do_symlink(s, vs);
>           v9fs_create_post_perms(s, vs, err);
>       } else if (vs->perm&  P9_STAT_MODE_LINK) {
>           int32_t nfid = atoi(vs->extension.data);
> @@ -1791,7 +1801,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
>               err = -errno;
>               v9fs_post_create(s, vs, err);
>           }
> -        err = v9fs_do_link(s,&nfidp->path,&vs->fullname);
> +        err = v9fs_do_link(s, nfidp, vs);
>           v9fs_create_post_perms(s, vs, err);
>       } else if (vs->perm&  P9_STAT_MODE_DEVICE) {
>           char ctype;
>
diff mbox

Patch

diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
index 752fbad..0a9c10a 100644
--- a/hw/file-op-9p.h
+++ b/hw/file-op-9p.h
@@ -54,8 +54,8 @@  typedef struct FileOperations
     int (*mknod)(FsContext *, const char *, FsCred *);
     int (*utime)(FsContext *, const char *, const struct utimbuf *);
     int (*remove)(FsContext *, const char *);
-    int (*symlink)(FsContext *, const char *, const char *);
-    int (*link)(FsContext *, const char *, const char *);
+    int (*symlink)(FsContext *, const char *, const char *, FsCred *);
+    int (*link)(FsContext *, const char *, const char *, FsCred *);
     int (*setuid)(FsContext *, uid_t);
     int (*close)(FsContext *, int);
     int (*closedir)(FsContext *, DIR *);
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index bfcd695..ca0f065 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -64,12 +64,25 @@  static int local_set_xattr(const char *path, FsCred *credp)
         }
     }
      return 0;
- }
+}
 
-static ssize_t local_readlink(FsContext *ctx, const char *path,
-                                char *buf, size_t bufsz)
+static ssize_t local_readlink(FsContext *fs_ctx, const char *path,
+        char *buf, size_t bufsz)
 {
-    return readlink(rpath(ctx, path), buf, bufsz);
+    ssize_t tsize = -1;
+    if (fs_ctx->fs_sm == sm_mapped) {
+        int fd;
+        fd = open(rpath(fs_ctx, path), O_RDONLY);
+        if (fd == -1) {
+            return -1;
+        }
+        tsize = read(fd, (void *)buf, bufsz);
+        close(fd);
+        return tsize;
+    } else if (fs_ctx->fs_sm == sm_passthrough) {
+        tsize = readlink(rpath(fs_ctx, path), buf, bufsz);
+    }
+    return tsize;
 }
 
 static int local_close(FsContext *ctx, int fd)
@@ -247,32 +260,70 @@  err_end:
     return err;
 }
 
-static int local_symlink(FsContext *ctx, const char *oldpath,
-                            const char *newpath)
+
+static int local_symlink(FsContext *fs_ctx, const char *oldpath,
+        const char *newpath, FsCred *credp)
 {
-    return symlink(oldpath, rpath(ctx, newpath));
+    int err = -1;
+    /* Determine the security model */
+    if (fs_ctx->fs_sm == sm_mapped) {
+        int fd;
+        ssize_t oldpath_size, write_size;
+        fd = open(rpath(fs_ctx, newpath), O_CREAT|O_EXCL|O_RDWR,
+                SM_LOCAL_MODE_BITS);
+        if (fd == -1) {
+            return fd;
+        }
+        /* Write the oldpath (target) to the file. */
+        oldpath_size = strlen(oldpath) + 1;
+        write_size = write(fd, (void *)oldpath, oldpath_size);
+        if (write_size != oldpath_size) {
+            close(fd);
+            err = -1;
+            goto err_end;
+        }
+        close(fd);
+        /* Set cleint credentials in symlink's xattr */
+        credp->fc_mode = credp->fc_mode|S_IFLNK;
+        err = local_set_xattr(rpath(fs_ctx, newpath), credp);
+        if (err == -1) {
+            goto err_end;
+        }
+    } else if (fs_ctx->fs_sm == sm_passthrough) {
+        err = symlink(oldpath, rpath(fs_ctx, newpath));
+        if (err) {
+            return err;
+        }
+        err = chmod(rpath(fs_ctx, newpath), credp->fc_mode & 07777);
+        if (err == -1) {
+            goto err_end;
+        }
+        err = chown(rpath(fs_ctx, newpath), credp->fc_uid, credp->fc_gid);
+        if (err == -1) {
+            goto err_end;
+        }
+    }
+    return err;
+
+err_end:
+    remove(rpath(fs_ctx, newpath));
+    return err;
 }
 
-static int local_link(FsContext *ctx, const char *oldpath, const char *newpath)
+static int local_link(FsContext *fs_ctx, const char *oldpath,
+        const char *newpath, FsCred *credp)
 {
-    char *tmp = qemu_strdup(rpath(ctx, oldpath));
-    int err, serrno = 0;
+    char *tmp = qemu_strdup(rpath(fs_ctx, oldpath));
+    int err;
 
     if (tmp == NULL) {
         return -ENOMEM;
     }
 
-    err = link(tmp, rpath(ctx, newpath));
-    if (err == -1) {
-        serrno = errno;
-    }
+    err = link(tmp, rpath(fs_ctx, newpath));
 
     qemu_free(tmp);
 
-    if (err == -1) {
-        errno = serrno;
-    }
-
     return err;
 }
 
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 30f649d..fbc846b 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -197,15 +197,25 @@  static int v9fs_do_open2(V9fsState *s, V9fsCreateState *vs)
     return s->ops->open2(&s->ctx, vs->fullname.data, flags, &cred);
 }
 
-static int v9fs_do_symlink(V9fsState *s, V9fsString *oldpath,
-                            V9fsString *newpath)
+static int v9fs_do_symlink(V9fsState *s, V9fsCreateState *vs)
 {
-    return s->ops->symlink(&s->ctx, oldpath->data, newpath->data);
+    FsCred cred;
+    cred_init(&cred);
+    cred.fc_uid = vs->fidp->uid;
+    cred.fc_mode = vs->perm | 0777;
+
+    return s->ops->symlink(&s->ctx, vs->extension.data, vs->fullname.data,
+            &cred);
 }
 
-static int v9fs_do_link(V9fsState *s, V9fsString *oldpath, V9fsString *newpath)
+static int v9fs_do_link(V9fsState *s, V9fsFidState *nfidp, V9fsCreateState *vs)
 {
-    return s->ops->link(&s->ctx, oldpath->data, newpath->data);
+    FsCred cred;
+    cred_init(&cred);
+    cred.fc_uid = nfidp->uid;
+    cred.fc_mode = vs->perm & 0777;
+
+    return s->ops->link(&s->ctx, nfidp->path.data, vs->fullname.data, &cred);
 }
 
 static int v9fs_do_truncate(V9fsState *s, V9fsString *path, off_t size)
@@ -1782,7 +1792,7 @@  static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
         err = v9fs_do_mkdir(s, vs);
         v9fs_create_post_mkdir(s, vs, err);
     } else if (vs->perm & P9_STAT_MODE_SYMLINK) {
-        err = v9fs_do_symlink(s, &vs->extension, &vs->fullname);
+        err = v9fs_do_symlink(s, vs);
         v9fs_create_post_perms(s, vs, err);
     } else if (vs->perm & P9_STAT_MODE_LINK) {
         int32_t nfid = atoi(vs->extension.data);
@@ -1791,7 +1801,7 @@  static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
             err = -errno;
             v9fs_post_create(s, vs, err);
         }
-        err = v9fs_do_link(s, &nfidp->path, &vs->fullname);
+        err = v9fs_do_link(s, nfidp, vs);
         v9fs_create_post_perms(s, vs, err);
     } else if (vs->perm & P9_STAT_MODE_DEVICE) {
         char ctype;