From 2c30dd744aa02d31a8a3b87daaba0b2cb774f346 Mon Sep 17 00:00:00 2001 From: "Aneesh Kumar K.V" Date: Thu, 19 Jan 2012 12:21:11 +0530 Subject: [PATCH 1/6] hw/9pfs: Add new security model mapped-file. This enable us to do passthrough equivalent security model on NFS directory. NFS server mostly do root squashing and don't support xattr. Hence we cannot use 'passthrough' or 'mapped' security model Also added "mapped-xattr" security to indicate earlier "mapped" security model Older name is still supported. POSIX rules regarding ctime update on chmod are not followed by this security model. Signed-off-by: Aneesh Kumar K.V --- fsdev/file-op-9p.h | 12 +- hw/9pfs/cofile.c | 14 ++ hw/9pfs/virtio-9p-device.c | 9 - hw/9pfs/virtio-9p-local.c | 350 ++++++++++++++++++++++++++++++++++++- qemu-options.hx | 18 +- 5 files changed, 378 insertions(+), 25 deletions(-) diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h index 1e96c8bed9..956fda0919 100644 --- a/fsdev/file-op-9p.h +++ b/fsdev/file-op-9p.h @@ -56,11 +56,15 @@ typedef struct extended_ops { * On failure ignore the error. */ #define V9FS_SM_NONE 0x00000010 -#define V9FS_RDONLY 0x00000020 -#define V9FS_PROXY_SOCK_FD 0x00000040 -#define V9FS_PROXY_SOCK_NAME 0x00000080 +/* + * uid/gid part of .virtfs_meatadata namespace + */ +#define V9FS_SM_MAPPED_FILE 0x00000020 +#define V9FS_RDONLY 0x00000040 +#define V9FS_PROXY_SOCK_FD 0x00000080 +#define V9FS_PROXY_SOCK_NAME 0x00000100 -#define V9FS_SEC_MASK 0x0000001C +#define V9FS_SEC_MASK 0x0000003C typedef struct FileOperations FileOperations; diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c index b15838c1e6..9345aaeb2e 100644 --- a/hw/9pfs/cofile.c +++ b/hw/9pfs/cofile.c @@ -76,6 +76,20 @@ int v9fs_co_fstat(V9fsPDU *pdu, V9fsFidState *fidp, struct stat *stbuf) err = -errno; } }); + /* + * Some FS driver (local:mapped-file) can't support fetching attributes + * using file descriptor. Use Path name in that case. + */ + if (err == -EOPNOTSUPP) { + err = v9fs_co_lstat(pdu, &fidp->path, stbuf); + if (err == -ENOENT) { + /* + * fstat on an unlinked file. Work with partial results + * returned from s->ops->fstat + */ + err = 0; + } + } return err; } diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 642d5e2c46..41a1a820e0 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -91,15 +91,6 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf) s->ctx.fs_root = NULL; } s->ctx.exops.get_st_gen = NULL; - - if (fse->export_flags & V9FS_SM_PASSTHROUGH) { - s->ctx.xops = passthrough_xattr_ops; - } else if (fse->export_flags & V9FS_SM_MAPPED) { - s->ctx.xops = mapped_xattr_ops; - } else if (fse->export_flags & V9FS_SM_NONE) { - s->ctx.xops = none_xattr_ops; - } - len = strlen(conf->tag); if (len > MAX_TAG_LEN - 1) { fprintf(stderr, "mount tag '%s' (%d bytes) is longer than " diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index 3ae6ef2e39..6e3f9d1d97 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -20,6 +20,7 @@ #include #include #include "qemu-xattr.h" +#include #include #ifdef CONFIG_LINUX_MAGIC_H #include @@ -39,6 +40,54 @@ #define BTRFS_SUPER_MAGIC 0x9123683E #endif +#define VIRTFS_META_DIR ".virtfs_metadata" + +static const char *local_mapped_attr_path(FsContext *ctx, + const char *path, char *buffer) +{ + char *dir_name; + char *tmp_path = strdup(path); + char *base_name = basename(tmp_path); + + /* NULL terminate the directory */ + dir_name = tmp_path; + *(base_name - 1) = '\0'; + + snprintf(buffer, PATH_MAX, "%s/%s/%s/%s", + ctx->fs_root, dir_name, VIRTFS_META_DIR, base_name); + free(tmp_path); + return buffer; +} + +#define ATTR_MAX 100 +static void local_mapped_file_attr(FsContext *ctx, const char *path, + struct stat *stbuf) +{ + FILE *fp; + char buf[ATTR_MAX]; + char attr_path[PATH_MAX]; + + local_mapped_attr_path(ctx, path, attr_path); + fp = fopen(attr_path, "r"); + if (!fp) { + return; + } + memset(buf, 0, ATTR_MAX); + while (fgets(buf, ATTR_MAX, fp)) { + if (!strncmp(buf, "virtfs.uid", 10)) { + stbuf->st_uid = atoi(buf+11); + } else if (!strncmp(buf, "virtfs.gid", 10)) { + stbuf->st_gid = atoi(buf+11); + } else if (!strncmp(buf, "virtfs.mode", 11)) { + stbuf->st_mode = atoi(buf+12); + } else if (!strncmp(buf, "virtfs.rdev", 11)) { + stbuf->st_rdev = atoi(buf+12); + } + memset(buf, 0, ATTR_MAX); + } + fclose(fp); +} + static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf) { int err; @@ -71,10 +120,103 @@ static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf) sizeof(dev_t)) > 0) { stbuf->st_rdev = tmp_dev; } + } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { + local_mapped_file_attr(fs_ctx, path, stbuf); } return err; } +static int local_create_mapped_attr_dir(FsContext *ctx, const char *path) +{ + int err; + char attr_dir[PATH_MAX]; + char *tmp_path = strdup(path); + + snprintf(attr_dir, PATH_MAX, "%s/%s/%s", + ctx->fs_root, dirname(tmp_path), VIRTFS_META_DIR); + + err = mkdir(attr_dir, 0700); + if (err < 0 && errno == EEXIST) { + err = 0; + } + free(tmp_path); + return err; +} + +static int local_set_mapped_file_attr(FsContext *ctx, + const char *path, FsCred *credp) +{ + FILE *fp; + int ret = 0; + char buf[ATTR_MAX]; + char attr_path[PATH_MAX]; + int uid = -1, gid = -1, mode = -1, rdev = -1; + + fp = fopen(local_mapped_attr_path(ctx, path, attr_path), "r"); + if (!fp) { + goto create_map_file; + } + memset(buf, 0, ATTR_MAX); + while (fgets(buf, ATTR_MAX, fp)) { + if (!strncmp(buf, "virtfs.uid", 10)) { + uid = atoi(buf+11); + } else if (!strncmp(buf, "virtfs.gid", 10)) { + gid = atoi(buf+11); + } else if (!strncmp(buf, "virtfs.mode", 11)) { + mode = atoi(buf+12); + } else if (!strncmp(buf, "virtfs.rdev", 11)) { + rdev = atoi(buf+12); + } + memset(buf, 0, ATTR_MAX); + } + fclose(fp); + goto update_map_file; + +create_map_file: + ret = local_create_mapped_attr_dir(ctx, path); + if (ret < 0) { + goto err_out; + } + +update_map_file: + fp = fopen(attr_path, "w"); + if (!fp) { + ret = -1; + goto err_out; + } + + if (credp->fc_uid != -1) { + uid = credp->fc_uid; + } + if (credp->fc_gid != -1) { + gid = credp->fc_gid; + } + if (credp->fc_mode != -1) { + mode = credp->fc_mode; + } + if (credp->fc_rdev != -1) { + rdev = credp->fc_rdev; + } + + + if (uid != -1) { + fprintf(fp, "virtfs.uid=%d\n", uid); + } + if (gid != -1) { + fprintf(fp, "virtfs.gid=%d\n", gid); + } + if (mode != -1) { + fprintf(fp, "virtfs.mode=%d\n", mode); + } + if (rdev != -1) { + fprintf(fp, "virtfs.rdev=%d\n", rdev); + } + fclose(fp); + +err_out: + return ret; +} + static int local_set_xattr(const char *path, FsCred *credp) { int err; @@ -138,7 +280,8 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_path, char buffer[PATH_MAX]; char *path = fs_path->data; - if (fs_ctx->export_flags & V9FS_SM_MAPPED) { + if ((fs_ctx->export_flags & V9FS_SM_MAPPED) || + (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE)) { int fd; fd = open(rpath(fs_ctx, path, buffer), O_RDONLY); if (fd == -1) { @@ -203,7 +346,18 @@ static int local_readdir_r(FsContext *ctx, V9fsFidOpenState *fs, struct dirent *entry, struct dirent **result) { - return readdir_r(fs->dir, entry, result); + int ret; + +again: + ret = readdir_r(fs->dir, entry, result); + if (ctx->export_flags & V9FS_SM_MAPPED_FILE) { + if (!ret && *result != NULL && + !strcmp(entry->d_name, VIRTFS_META_DIR)) { + /* skp the meta data directory */ + goto again; + } + } + return ret; } static void local_seekdir(FsContext *ctx, V9fsFidOpenState *fs, off_t off) @@ -264,6 +418,8 @@ static int local_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp) if (fs_ctx->export_flags & V9FS_SM_MAPPED) { return local_set_xattr(rpath(fs_ctx, path, buffer), credp); + } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { + return local_set_mapped_file_attr(fs_ctx, path, credp); } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || (fs_ctx->export_flags & V9FS_SM_NONE)) { return chmod(rpath(fs_ctx, path, buffer), credp->fc_mode); @@ -296,6 +452,18 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, serrno = errno; goto err_end; } + } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { + + err = mknod(rpath(fs_ctx, path, buffer), + SM_LOCAL_MODE_BITS|S_IFREG, 0); + if (err == -1) { + goto out; + } + err = local_set_mapped_file_attr(fs_ctx, path, credp); + if (err == -1) { + serrno = errno; + goto err_end; + } } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || (fs_ctx->export_flags & V9FS_SM_NONE)) { err = mknod(rpath(fs_ctx, path, buffer), credp->fc_mode, @@ -344,6 +512,17 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, serrno = errno; goto err_end; } + } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { + err = mkdir(rpath(fs_ctx, path, buffer), SM_LOCAL_DIR_MODE_BITS); + if (err == -1) { + goto out; + } + credp->fc_mode = credp->fc_mode|S_IFDIR; + err = local_set_mapped_file_attr(fs_ctx, path, credp); + if (err == -1) { + serrno = errno; + goto err_end; + } } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || (fs_ctx->export_flags & V9FS_SM_NONE)) { err = mkdir(rpath(fs_ctx, path, buffer), credp->fc_mode); @@ -404,6 +583,9 @@ static int local_fstat(FsContext *fs_ctx, int fid_type, &tmp_dev, sizeof(dev_t)) > 0) { stbuf->st_rdev = tmp_dev; } + } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { + errno = EOPNOTSUPP; + return -1; } return err; } @@ -436,6 +618,19 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, serrno = errno; goto err_end; } + } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { + fd = open(rpath(fs_ctx, path, buffer), flags, SM_LOCAL_MODE_BITS); + if (fd == -1) { + err = fd; + goto out; + } + credp->fc_mode = credp->fc_mode|S_IFREG; + /* Set client credentials in .virtfs_metadata directory files */ + err = local_set_mapped_file_attr(fs_ctx, path, credp); + if (err == -1) { + serrno = errno; + goto err_end; + } } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || (fs_ctx->export_flags & V9FS_SM_NONE)) { fd = open(rpath(fs_ctx, path, buffer), flags, credp->fc_mode); @@ -506,6 +701,35 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, serrno = errno; goto err_end; } + } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { + int fd; + ssize_t oldpath_size, write_size; + fd = open(rpath(fs_ctx, newpath, buffer), O_CREAT|O_EXCL|O_RDWR, + 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; + } + 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; + } } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || (fs_ctx->export_flags & V9FS_SM_NONE)) { err = symlink(oldpath, rpath(fs_ctx, newpath, buffer)); @@ -548,6 +772,21 @@ static int local_link(FsContext *ctx, V9fsPath *oldpath, ret = link(rpath(ctx, oldpath->data, buffer), rpath(ctx, newpath.data, buffer1)); + + /* now link the virtfs_metadata files */ + if (!ret && (ctx->export_flags & V9FS_SM_MAPPED_FILE)) { + /* Link the .virtfs_metadata files. Create the metada directory */ + ret = local_create_mapped_attr_dir(ctx, newpath.data); + if (ret < 0) { + goto err_out; + } + ret = link(local_mapped_attr_path(ctx, oldpath->data, buffer), + local_mapped_attr_path(ctx, newpath.data, buffer1)); + if (ret < 0 && errno != ENOENT) { + goto err_out; + } + } +err_out: v9fs_string_free(&newpath); return ret; } @@ -563,8 +802,21 @@ static int local_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size) static int local_rename(FsContext *ctx, const char *oldpath, const char *newpath) { + int err; char buffer[PATH_MAX], buffer1[PATH_MAX]; + if (ctx->export_flags & V9FS_SM_MAPPED_FILE) { + err = local_create_mapped_attr_dir(ctx, newpath); + if (err < 0) { + return err; + } + /* rename the .virtfs_metadata files */ + err = rename(local_mapped_attr_path(ctx, oldpath, buffer), + local_mapped_attr_path(ctx, newpath, buffer1)); + if (err < 0 && errno != ENOENT) { + return err; + } + } return rename(rpath(ctx, oldpath, buffer), rpath(ctx, newpath, buffer1)); } @@ -580,6 +832,8 @@ static int local_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp) credp->fc_uid, credp->fc_gid); } else if (fs_ctx->export_flags & V9FS_SM_MAPPED) { return local_set_xattr(rpath(fs_ctx, path, buffer), credp); + } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { + return local_set_mapped_file_attr(fs_ctx, path, credp); } return -1; } @@ -595,8 +849,46 @@ static int local_utimensat(FsContext *s, V9fsPath *fs_path, static int local_remove(FsContext *ctx, const char *path) { + int err; + struct stat stbuf; char buffer[PATH_MAX]; + + if (ctx->export_flags & V9FS_SM_MAPPED_FILE) { + err = lstat(rpath(ctx, path, buffer), &stbuf); + if (err) { + goto err_out; + } + /* + * If directory remove .virtfs_metadata contained in the + * directory + */ + if (S_ISDIR(stbuf.st_mode)) { + sprintf(buffer, "%s/%s/%s", ctx->fs_root, path, VIRTFS_META_DIR); + err = remove(buffer); + if (err < 0 && errno != ENOENT) { + /* + * We didn't had the .virtfs_metadata file. May be file created + * in non-mapped mode ?. Ignore ENOENT. + */ + goto err_out; + } + } + /* + * Now remove the name from parent directory + * .virtfs_metadata directory + */ + err = remove(local_mapped_attr_path(ctx, path, buffer));; + if (err < 0 && errno != ENOENT) { + /* + * We didn't had the .virtfs_metadata file. May be file created + * in non-mapped mode ?. Ignore ENOENT. + */ + goto err_out; + } + } return remove(rpath(ctx, path, buffer)); +err_out: + return err; } static int local_fsync(FsContext *ctx, int fid_type, @@ -696,12 +988,45 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir, int ret; V9fsString fullname; char buffer[PATH_MAX]; + v9fs_string_init(&fullname); v9fs_string_sprintf(&fullname, "%s/%s", dir->data, name); + if (ctx->export_flags & V9FS_SM_MAPPED_FILE) { + if (flags == AT_REMOVEDIR) { + /* + * If directory remove .virtfs_metadata contained in the + * directory + */ + sprintf(buffer, "%s/%s/%s", ctx->fs_root, + fullname.data, VIRTFS_META_DIR); + ret = remove(buffer); + if (ret < 0 && errno != ENOENT) { + /* + * We didn't had the .virtfs_metadata file. May be file created + * in non-mapped mode ?. Ignore ENOENT. + */ + goto err_out; + } + } + /* + * Now remove the name from parent directory + * .virtfs_metadata directory. + */ + ret = remove(local_mapped_attr_path(ctx, fullname.data, buffer)); + if (ret < 0 && errno != ENOENT) { + /* + * We didn't had the .virtfs_metadata file. May be file created + * in non-mapped mode ?. Ignore ENOENT. + */ + goto err_out; + } + } + /* Remove the name finally */ ret = remove(rpath(ctx, fullname.data, buffer)); v9fs_string_free(&fullname); +err_out: return ret; } @@ -736,6 +1061,19 @@ static int local_init(FsContext *ctx) int err = 0; struct statfs stbuf; + if (ctx->export_flags & V9FS_SM_PASSTHROUGH) { + ctx->xops = passthrough_xattr_ops; + } else if (ctx->export_flags & V9FS_SM_MAPPED) { + ctx->xops = mapped_xattr_ops; + } else if (ctx->export_flags & V9FS_SM_NONE) { + ctx->xops = none_xattr_ops; + } else if (ctx->export_flags & V9FS_SM_MAPPED_FILE) { + /* + * xattr operation for mapped-file and passthrough + * remain same. + */ + ctx->xops = passthrough_xattr_ops; + } ctx->export_flags |= V9FS_PATHNAME_FSCONTEXT; #ifdef FS_IOC_GETVERSION /* @@ -770,13 +1108,17 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse) if (!strcmp(sec_model, "passthrough")) { fse->export_flags |= V9FS_SM_PASSTHROUGH; - } else if (!strcmp(sec_model, "mapped")) { + } else if (!strcmp(sec_model, "mapped") || + !strcmp(sec_model, "mapped-xattr")) { fse->export_flags |= V9FS_SM_MAPPED; } else if (!strcmp(sec_model, "none")) { fse->export_flags |= V9FS_SM_NONE; + } else if (!strcmp(sec_model, "mapped-file")) { + fse->export_flags |= V9FS_SM_MAPPED_FILE; } else { fprintf(stderr, "Invalid security model %s specified, valid options are" - "\n\t [passthrough|mapped|none]\n", sec_model); + "\n\t [passthrough|mapped-xattr|mapped-file|none]\n", + sec_model); return -1; } diff --git a/qemu-options.hx b/qemu-options.hx index 6295cde351..0cefd1852f 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -551,7 +551,7 @@ DEFHEADING() DEFHEADING(File system options:) DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev, - "-fsdev fsdriver,id=id[,path=path,][security_model={mapped|passthrough|none}]\n" + "-fsdev fsdriver,id=id[,path=path,][security_model={mapped-xattr|mapped-file|passthrough|none}]\n" " [,writeout=immediate][,readonly][,socket=socket|sock_fd=sock_fd]\n", QEMU_ARCH_ALL) @@ -571,12 +571,13 @@ Specifies the export path for the file system device. Files under this path will be available to the 9p client on the guest. @item security_model=@var{security_model} Specifies the security model to be used for this export path. -Supported security models are "passthrough", "mapped" and "none". +Supported security models are "passthrough", "mapped-xattr", "mapped-file" and "none". In "passthrough" security model, files are stored using the same credentials as they are created on the guest. This requires qemu -to run as root. In "mapped" security model, some of the file +to run as root. In "mapped-xattr" security model, some of the file attributes like uid, gid, mode bits and link target are stored as -file attributes. Directories exported by this security model cannot +file attributes. For "mapped-file" these attributes are stored in the +hidden .virtfs_metadata directory. Directories exported by this security model cannot interact with other unix tools. "none" security model is same as passthrough except the sever won't report failures if it fails to set file attributes like ownership. Security model is mandatory @@ -616,7 +617,7 @@ DEFHEADING() DEFHEADING(Virtual File system pass-through options:) DEF("virtfs", HAS_ARG, QEMU_OPTION_virtfs, - "-virtfs local,path=path,mount_tag=tag,security_model=[mapped|passthrough|none]\n" + "-virtfs local,path=path,mount_tag=tag,security_model=[mapped-xattr|mapped-file|passthrough|none]\n" " [,writeout=immediate][,readonly][,socket=socket|sock_fd=sock_fd]\n", QEMU_ARCH_ALL) @@ -637,12 +638,13 @@ Specifies the export path for the file system device. Files under this path will be available to the 9p client on the guest. @item security_model=@var{security_model} Specifies the security model to be used for this export path. -Supported security models are "passthrough", "mapped" and "none". +Supported security models are "passthrough", "mapped-xattr", "mapped-file" and "none". In "passthrough" security model, files are stored using the same credentials as they are created on the guest. This requires qemu -to run as root. In "mapped" security model, some of the file +to run as root. In "mapped-xattr" security model, some of the file attributes like uid, gid, mode bits and link target are stored as -file attributes. Directories exported by this security model cannot +file attributes. For "mapped-file" these attributes are stored in the +hidden .virtfs_metadata directory. Directories exported by this security model cannot interact with other unix tools. "none" security model is same as passthrough except the sever won't report failures if it fails to set file attributes like ownership. Security model is mandatory only From 2d40564aaab3a99fe6ce00fc0fc893c02e9443ec Mon Sep 17 00:00:00 2001 From: "M. Mohan Kumar" Date: Thu, 19 Jan 2012 12:21:12 +0530 Subject: [PATCH 2/6] hw/9pfs: Preserve S_ISGID In passthrough security model in local fs driver, after a file creation chown and chmod are done to set the file credentials and mode as requested by 9p client. But if there was a request to create a file with S_ISGID bit, doing chown on that file resets the S_ISGID bit. So first call chown and then invoking chmod with proper mode bit retains the S_ISGID (if present/requested) This resulted in LTP mknod02, mknod03, mknod05, open10 test case failures. This patch fixes this issue. man 2 chown When the owner or group of an executable file are changed by an unprivileged user the S_ISUID and S_ISGID mode bits are cleared. POSIX does not specify whether this also should happen when root does the chown(); the Linux behavior depends on the kernel version. Signed-off-by: M. Mohan Kumar Signed-off-by: Aneesh Kumar K.V --- hw/9pfs/virtio-9p-handle.c | 4 ++-- hw/9pfs/virtio-9p-local.c | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c index cb012c0510..f96d17a974 100644 --- a/hw/9pfs/virtio-9p-handle.c +++ b/hw/9pfs/virtio-9p-handle.c @@ -63,11 +63,11 @@ static int handle_update_file_cred(int dirfd, const char *name, FsCred *credp) if (fd < 0) { return fd; } - ret = fchmod(fd, credp->fc_mode & 07777); + ret = fchownat(fd, "", credp->fc_uid, credp->fc_gid, AT_EMPTY_PATH); if (ret < 0) { goto err_out; } - ret = fchownat(fd, "", credp->fc_uid, credp->fc_gid, AT_EMPTY_PATH); + ret = fchmod(fd, credp->fc_mode & 07777); err_out: close(fd); return ret; diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index 6e3f9d1d97..33a41d2e18 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -257,9 +257,6 @@ static int local_post_create_passthrough(FsContext *fs_ctx, const char *path, { char buffer[PATH_MAX]; - if (chmod(rpath(fs_ctx, path, buffer), credp->fc_mode & 07777) < 0) { - return -1; - } if (lchown(rpath(fs_ctx, path, buffer), credp->fc_uid, credp->fc_gid) < 0) { /* @@ -270,6 +267,10 @@ static int local_post_create_passthrough(FsContext *fs_ctx, const char *path, return -1; } } + + if (chmod(rpath(fs_ctx, path, buffer), credp->fc_mode & 07777) < 0) { + return -1; + } return 0; } From 71f86cd6f396fe3ede575d7dde5dbabb4a6d006e Mon Sep 17 00:00:00 2001 From: "Aneesh Kumar K.V" Date: Thu, 19 Jan 2012 12:21:12 +0530 Subject: [PATCH 3/6] hw/9pfs: Fix crash when mounting with synthfs Some Fsdriver backend don't have fs_root. So check for that in migrate message. Signed-off-by: Aneesh Kumar K.V --- hw/9pfs/virtio-9p.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c index e6ba6ba30b..dfe2025ed2 100644 --- a/hw/9pfs/virtio-9p.c +++ b/hw/9pfs/virtio-9p.c @@ -986,7 +986,7 @@ static void v9fs_attach(void *opaque) s->root_fid = fid; /* disable migration */ error_set(&s->migration_blocker, QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION, - s->ctx.fs_root, s->tag); + s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag); migrate_add_blocker(s->migration_blocker); out: put_fid(pdu, fidp); From 5fc6dbae7416d7be38565391d4c213fa9085c9fb Mon Sep 17 00:00:00 2001 From: "M. Mohan Kumar" Date: Thu, 19 Jan 2012 16:15:56 +0530 Subject: [PATCH 4/6] fsdev: Fix parameter parsing for proxy helper This fixes a crash when using sockfd with proxy FsDriver Signed-off-by: M. Mohan Kumar Signed-off-by: Aneesh Kumar K.V --- fsdev/virtfs-proxy-helper.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c index 4a507d860e..f9a8270ee9 100644 --- a/fsdev/virtfs-proxy-helper.c +++ b/fsdev/virtfs-proxy-helper.c @@ -1036,7 +1036,13 @@ int main(int argc, char **argv) return -1; } - if (*sock_name && (own_u == -1 || own_g == -1)) { + if (sock_name && sock != -1) { + fprintf(stderr, "both named socket and socket descriptor specified\n"); + usage(argv[0]); + exit(EXIT_FAILURE); + } + + if (sock_name && (own_u == -1 || own_g == -1)) { fprintf(stderr, "owner uid:gid not specified, "); fprintf(stderr, "owner uid:gid specifies who can access the socket file\n"); @@ -1064,7 +1070,7 @@ int main(int argc, char **argv) } do_log(LOG_INFO, "Started\n"); - if (*sock_name) { + if (sock_name) { sock = proxy_socket(sock_name, own_u, own_g); if (sock < 0) { goto error; From 68e59e14605f9e6390d8d975f9ab919be9176bc2 Mon Sep 17 00:00:00 2001 From: "Aneesh Kumar K.V" Date: Sun, 15 Jan 2012 23:31:04 +0530 Subject: [PATCH 5/6] hw/9pfs: Update MAINTAINERS file Acked-by: Venkateswararao Jujjuri Signed-off-by: Aneesh Kumar K.V --- MAINTAINERS | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index de2a9163f5..f9f131c305 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -399,9 +399,11 @@ S: Supported F: hw/virtio* virtio-9p -M: Venkateswararao Jujjuri (JV) +M: Aneesh Kumar K.V S: Supported -F: hw/virtio-9p* +F: hw/9pfs/ fsdev/ +T: https://github.com/kvaneesh/QEMU + virtio-blk M: Kevin Wolf From eed968607d656a218712df47a5e0432c21fd6994 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 16 Jan 2012 18:11:40 +0000 Subject: [PATCH 6/6] hw/9pfs: Remove O_NOATIME flag from 9pfs open() calls in readonly mode When 2c74c2cb4bedddbfa67628fbd5f9273b4e0e9903 added support for the 'readonly' flag against 9p filesystems, it also made QEMU add the O_NOATIME flag as a side-effect. The O_NOATIME flag, however, may only be set by the file owner, or a user with CAP_FOWNER capability. QEMU cannot assume that this is the case for filesytems exported to QEMU. eg, run QEMU as non-root, and attempt to pass the host OS filesystem through to the guest OS with readonly enable. The result is that the guest OS cannot open any files at all. If O_NOATIME is really required, it should be optionally enabled via a separate QEMU command line flag. * hw/9pfs/virtio-9p.c: Remove O_NOATIME Acked-by: M. Mohan Kumar Signed-off-by: Daniel P. Berrange Signed-off-by: Aneesh Kumar K.V --- hw/9pfs/virtio-9p.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c index dfe2025ed2..a72ffc3390 100644 --- a/hw/9pfs/virtio-9p.c +++ b/hw/9pfs/virtio-9p.c @@ -1391,7 +1391,6 @@ static void v9fs_open(void *opaque) err = -EROFS; goto out; } - flags |= O_NOATIME; } err = v9fs_co_open(pdu, fidp, flags); if (err < 0) {