[Qemu-devel] [PATCH RFC 06/36] 9pfs: local: split symlink operation per security model

[Greg Kurz]

Having all security models implemented in one monolithic function is
cumbersome. Especially when the need arises to fix something in the
shared code, as it forces to change all the paths at the same time.

This doesn't fix any bug, it is just preparatory cleanup.

Signed-off-by: Greg Kurz <address@hidden>
---
 hw/9pfs/9p-local.c |  210 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 137 insertions(+), 73 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 4deb19ce8775..f9abd4229b17 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -941,96 +941,144 @@ static int local_open2(FsContext *fs_ctx, V9fsPath 
*dir_path, const char *name,
     g_assert_not_reached();
 }
 
-static int local_symlink(FsContext *fs_ctx, const char *oldpath,
-                         V9fsPath *dir_path, const char *name, FsCred *credp)
+static int local_symlink_mapped(FsContext *fs_ctx, const char *oldpath,
+                                V9fsPath *dir_path, const char *name,
+                                FsCred *credp)
 {
     int err = -1;
     int serrno = 0;
     char *newpath;
     V9fsString fullname;
     char *buffer = NULL;
+    int fd;
+    ssize_t oldpath_size, write_size;
 
     v9fs_string_init(&fullname);
     v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
     newpath = fullname.data;
 
-    /* Determine the security model */
-    if (fs_ctx->export_flags & V9FS_SM_MAPPED) {
-        int fd;
-        ssize_t oldpath_size, write_size;
-        buffer = rpath(fs_ctx, newpath);
-        fd = open(buffer, O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, 
SM_LOCAL_MODE_BITS);
-        if (fd == -1) {
-            err = fd;
-            goto out;
-        }
-        /* Write the oldpath (target) to the file. */
-        oldpath_size = strlen(oldpath);
-        do {
-            write_size = write(fd, (void *)oldpath, oldpath_size);
-        } while (write_size == -1 && errno == EINTR);
+    buffer = rpath(fs_ctx, newpath);
+    fd = open(buffer, O_CREAT | O_EXCL | O_RDWR | O_NOFOLLOW,
+              SM_LOCAL_MODE_BITS);
+    if (fd == -1) {
+        err = fd;
+        goto out;
+    }
+    /* Write the oldpath (target) to the file. */
+    oldpath_size = strlen(oldpath);
+    do {
+        write_size = write(fd, (void *)oldpath, oldpath_size);
+    } while (write_size == -1 && errno == EINTR);
 
-        if (write_size != oldpath_size) {
-            serrno = errno;
-            close(fd);
-            err = -1;
-            goto err_end;
-        }
+    if (write_size != oldpath_size) {
+        serrno = errno;
         close(fd);
-        /* Set cleint credentials in symlink's xattr */
-        credp->fc_mode = credp->fc_mode|S_IFLNK;
-        err = local_set_xattr(buffer, credp);
-        if (err == -1) {
-            serrno = errno;
-            goto err_end;
-        }
-    } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
-        int fd;
-        ssize_t oldpath_size, write_size;
-        buffer = rpath(fs_ctx, newpath);
-        fd = open(buffer, O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, 
SM_LOCAL_MODE_BITS);
-        if (fd == -1) {
-            err = fd;
-            goto out;
-        }
-        /* Write the oldpath (target) to the file. */
-        oldpath_size = strlen(oldpath);
-        do {
-            write_size = write(fd, (void *)oldpath, oldpath_size);
-        } while (write_size == -1 && errno == EINTR);
+        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(buffer, credp);
+    if (err == -1) {
+        serrno = errno;
+        goto err_end;
+    }
+    goto out;
 
-        if (write_size != oldpath_size) {
-            serrno = errno;
-            close(fd);
-            err = -1;
-            goto err_end;
-        }
+err_end:
+    remove(buffer);
+    errno = serrno;
+out:
+    g_free(buffer);
+    v9fs_string_free(&fullname);
+    return err;
+}
+
+static int local_symlink_mapped_file(FsContext *fs_ctx, const char *oldpath,
+                                     V9fsPath *dir_path, const char *name,
+                                     FsCred *credp)
+{
+    int err = -1;
+    int serrno = 0;
+    char *newpath;
+    V9fsString fullname;
+    char *buffer = NULL;
+    int fd;
+    ssize_t oldpath_size, write_size;
+
+    v9fs_string_init(&fullname);
+    v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
+    newpath = fullname.data;
+
+    buffer = rpath(fs_ctx, newpath);
+    fd = open(buffer, O_CREAT | O_EXCL | O_RDWR | O_NOFOLLOW,
+              SM_LOCAL_MODE_BITS);
+    if (fd == -1) {
+        err = fd;
+        goto out;
+    }
+    /* Write the oldpath (target) to the file. */
+    oldpath_size = strlen(oldpath);
+    do {
+        write_size = write(fd, (void *)oldpath, oldpath_size);
+    } while (write_size == -1 && errno == EINTR);
+
+    if (write_size != oldpath_size) {
+        serrno = errno;
         close(fd);
-        /* Set cleint credentials in symlink's xattr */
-        credp->fc_mode = credp->fc_mode|S_IFLNK;
-        err = local_set_mapped_file_attr(fs_ctx, newpath, credp);
-        if (err == -1) {
+        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_mapped_file_attr(fs_ctx, newpath, credp);
+    if (err == -1) {
+        serrno = errno;
+        goto err_end;
+    }
+    goto out;
+
+err_end:
+    remove(buffer);
+    errno = serrno;
+out:
+    g_free(buffer);
+    v9fs_string_free(&fullname);
+    return err;
+}
+
+static int local_symlink_passthrough(FsContext *fs_ctx, const char *oldpath,
+                                     V9fsPath *dir_path, const char *name,
+                                     FsCred *credp)
+{
+    int err = -1;
+    int serrno = 0;
+    char *newpath;
+    V9fsString fullname;
+    char *buffer = NULL;
+
+    v9fs_string_init(&fullname);
+    v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
+    newpath = fullname.data;
+
+    buffer = rpath(fs_ctx, newpath);
+    err = symlink(oldpath, buffer);
+    if (err) {
+        goto out;
+    }
+    err = lchown(buffer, credp->fc_uid, credp->fc_gid);
+    if (err == -1) {
+        /*
+         * If we fail to change ownership and if we are
+         * using security model none. Ignore the error
+         */
+        if ((fs_ctx->export_flags & V9FS_SEC_MASK) != V9FS_SM_NONE) {
             serrno = errno;
             goto err_end;
-        }
-    } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) ||
-               (fs_ctx->export_flags & V9FS_SM_NONE)) {
-        buffer = rpath(fs_ctx, newpath);
-        err = symlink(oldpath, buffer);
-        if (err) {
-            goto out;
-        }
-        err = lchown(buffer, credp->fc_uid, credp->fc_gid);
-        if (err == -1) {
-            /*
-             * If we fail to change ownership and if we are
-             * using security model none. Ignore the error
-             */
-            if ((fs_ctx->export_flags & V9FS_SEC_MASK) != V9FS_SM_NONE) {
-                serrno = errno;
-                goto err_end;
-            } else
-                err = 0;
+        } else {
+            err = 0;
         }
     }
     goto out;
@@ -1044,6 +1092,22 @@ out:
     return err;
 }
 
+static int local_symlink(FsContext *fs_ctx, const char *oldpath,
+                         V9fsPath *dir_path, const char *name, FsCred *credp)
+{
+    if (fs_ctx->export_flags & V9FS_SM_MAPPED) {
+        return local_symlink_mapped(fs_ctx, oldpath, dir_path, name, credp);
+    } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
+        return local_symlink_mapped_file(fs_ctx, oldpath, dir_path, name,
+                                         credp);
+    } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) ||
+               (fs_ctx->export_flags & V9FS_SM_NONE)) {
+        return local_symlink_passthrough(fs_ctx, oldpath, dir_path, name,
+                                         credp);
+    }
+    g_assert_not_reached();
+}
+
 static int local_link(FsContext *ctx, V9fsPath *oldpath,
                       V9fsPath *dirpath, const char *name)
 {