| Message ID | 53097D8E.1030803@gmail.com |
|---|---|
| State | New |
| Headers | show |
During the test, I found that if the path is long, the performance is very very slow under 9pfs (whether apply this patch or not, the results are the same). So after this patch passes checking, I will/should analyse this performance issue, next. Thanks. On 02/23/2014 12:48 PM, Chen Gang wrote: > When path is truncated by PATH_MAX limitation, it causes QEMU to access > incorrect file. So use original full path instead of PATH_MAX within > 9pfs (need check/process ENOMEM for related memory allocation). > > Also find/fix several another related issues when failure occurs. > > - check 'fh' in handle_name_to_path(). > > - need call v9fs_string_free() at 'err_out:' in local_unlinkat(). > > - sprintf() will cause memory overflow in "virtio-9p-local.c" > > > The related test: > > - Environments (for qemu-devel): > > - Host is under fedora17 desktop with ext4fs: > > qemu-system-x86_64 -hda test.img -m 1024 \ > -net nic,vlan=4,model=virtio,macaddr=00:16:35:AF:94:04 \ > -net tap,vlan=4,ifname=tap4,script=no,downscript=no \ > -device virtio-9p-pci,id=fs0,fsdev=fsdev0,mount_tag=hostshare \ > -fsdev local,security_model=passthrough,id=fsdev0,\ > path=/upstream/vm/data/share/1234567890abcdefghijklmnopqrstuvwxyz\ > ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890acdefghijklmnopqrstuvwxyz\ > ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890/111111111111111111111111111\ > 1111111111111111111111111111111111111111111111111111222222222222\ > 2222222222222222222222222222222222222222222222222222222222222222\ > 2222222222222222222222222222222222233333333333333333333333333333\ > 3333333333333333333333333333333333 > > - Guest is ubuntu12 server with 9pfs. > > mount -t 9p -o trans=virtio,version=9p2000.L hostshare /share > > - Limitations: > > full path limitation is PATH_MAX (4096B include nul) under Linux. > file/dir node name maximized length is 256 (include nul) under ext4. > > - Special test: > > Under host, modify the file: "/upstream/vm/data/share/1234567890abcdefg\ > hijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890acdefghijklmno\ > pqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890/111111111111111111111\ > 111111111111111111111111111111111111111111111111111111111122222222222\ > 222222222222222222222222222222222222222222222222222222222222222222222\ > 222222222222222222222222222222233333333333333333333333333333333333333\ > 3333333333333333333333333/4444444444444444444444444444444444444444444\ > 444444444444444444444444444444444444444444444444444444444444444444444\ > 444444444444444444444444444444444444444444444444444444444444444444444\ > 444444444444444444444444444444444444444/55555555555555555555555555555\ > 555555555555555555555555555555555555555555555555555555555555555555555\ > 555555555555555555555555555555555555555555555555555555555555555555555\ > 555555555555555555555555555555555555555555555555555555555555555555555\ > 55555555/666666666666666666666666666666666666666666666666666666666666\ > 666666666666666666666666666666666666666666666666666666666666666666666\ > 666666666666666666666666666666666666666666666666666666666666666666666\ > 666666666666666666666/77777777777777777777777777777777777777777777777\ > 777777777777777777777777777777777777777777777777777777777777777777777\ > 777777777777777777777777777777777777777777777777777777777777777777777\ > 77777777777777777777777777777777777777777777777777777777777/888888888\ > 888888888888888888888888888888888888888888888888888888888888888888888\ > 888888888888888888888888888888888888888888888888888888888888888888888\ > 888888888888888888888888888888888888888888888888888888888888888888888\ > 888888888/99999999999999999999999999999999999999999999999999999999999\ > 999999999999999999999999999999999999999999999999999999999999999999999\ > 999999999999999999999999999999999999999999999999999999999999999999999\ > 99999999999999999999999999999999999999999/000000000000000000000000000\ > 000000000000000000000000000000000000000000000000000000000000000000000\ > 000000000000000000000000000000000000000000000000000000000000000000000\ > 000000000000000000000000000000000000000000000000/aaaaaaaaaaaaaaaaaaaa\ > aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ > aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ > aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbb\ > bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\ > bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\ > bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/ccccccccc\ > ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc\ > ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc\ > ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc\ > cccccccccc/dddddddddddddddddddddddddddddddddddddddddddddddddddddddddd\ > ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd\ > ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd\ > dddddddddddddddddddddd/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\ > eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\ > eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\ > eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/fffffffffffffff\ > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ > fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff/gggggggggg\ > ggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg\ > ggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg\ > ggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg\ > ggggggggggggggggggggggg/iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii\ > iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii\ > iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii\ > iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii/jjjjjjjjjjjjj\ > jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj\ > jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj/ppppppppppppppppppppp\ > ppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppp\ > ppppppppppppppppppppppppppppppppppppppp/test1234567890file.log" > (need enter dir firstly, then modify file, or can not open it). > > Under guest, still allow modify "test1234567890file.log" (will generate > "test123456" file with contents). > > After apply this patch, can not open "test1234567890file.log" under guest > (permission denied). > > > - Common test: > > All are still OK after apply this path. > > "mkdir -p", "create/open file/dir", "modify file/dir", "rm file/dir". > change various mount point paths under host and/or guest. > > > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> > --- > hw/9pfs/cofs.c | 22 ++- > hw/9pfs/virtio-9p-handle.c | 25 ++- > hw/9pfs/virtio-9p-local.c | 433 +++++++++++++++++++++++++++++++---------- > hw/9pfs/virtio-9p-posix-acl.c | 76 ++++++-- > hw/9pfs/virtio-9p-xattr-user.c | 36 +++- > hw/9pfs/virtio-9p-xattr.c | 13 +- > hw/9pfs/virtio-9p-xattr.h | 39 +++- > hw/9pfs/virtio-9p.h | 6 +- > 8 files changed, 504 insertions(+), 146 deletions(-) > > diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c > index 3891050..ba69965 100644 > --- a/hw/9pfs/cofs.c > +++ b/hw/9pfs/cofs.c > @@ -20,18 +20,31 @@ > int v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf) > { > int err; > - ssize_t len; > + ssize_t len, maxlen = PATH_MAX; > V9fsState *s = pdu->s; > > if (v9fs_request_cancelled(pdu)) { > return -EINTR; > } > - buf->data = g_malloc(PATH_MAX); > + buf->data = g_malloc(maxlen); > + if (!buf->data) { > + return -ENOMEM; > + } > v9fs_path_read_lock(s); > v9fs_co_run_in_worker( > - { > + while (1) { > len = s->ops->readlink(&s->ctx, path, > - buf->data, PATH_MAX - 1); > + buf->data, maxlen - 1); > + if (len == maxlen - 1) { > + g_free(buf->data); > + maxlen *= 2; > + buf->data = g_malloc(maxlen); > + if (!buf->data) { > + err = -errno; > + break; > + } > + continue; > + } > if (len > -1) { > buf->size = len; > buf->data[len] = 0; > @@ -39,6 +52,7 @@ int v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf) > } else { > err = -errno; > } > + break; > }); > v9fs_path_unlock(s); > if (err) { > diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c > index fe8e0ed..fb19fb0 100644 > --- a/hw/9pfs/virtio-9p-handle.c > +++ b/hw/9pfs/virtio-9p-handle.c > @@ -498,7 +498,7 @@ static int handle_lremovexattr(FsContext *ctx, V9fsPath *fs_path, > static int handle_name_to_path(FsContext *ctx, V9fsPath *dir_path, > const char *name, V9fsPath *target) > { > - char buffer[PATH_MAX]; > + char *buffer; > struct file_handle *fh; > int dirfd, ret, mnt_id; > struct handle_data *data = (struct handle_data *)ctx->private; > @@ -513,15 +513,33 @@ static int handle_name_to_path(FsContext *ctx, V9fsPath *dir_path, > dirfd = open_by_handle(data->mountfd, dir_path->data, O_PATH); > } else { > /* relative to export root */ > - dirfd = open(rpath(ctx, ".", buffer), O_DIRECTORY); > + buffer = rpath(ctx, "."); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + dirfd = open(buffer, O_DIRECTORY); > + g_free(buffer); > } > if (dirfd < 0) { > return dirfd; > } > + > fh = g_malloc(sizeof(struct file_handle) + data->handle_bytes); > + if (!fh) { > + close(dirfd); > + errno = ENOMEM; > + return -1; > + } > fh->handle_bytes = data->handle_bytes; > /* add a "./" at the beginning of the path */ > - snprintf(buffer, PATH_MAX, "./%s", name); > + buffer = g_strdup_printf("./%s", name); > + if (!buffer) { > + g_free(fh); > + close(dirfd); > + errno = ENOMEM; > + return -1; > + } > /* flag = 0 imply don't follow symlink */ > ret = name_to_handle(dirfd, buffer, fh, &mnt_id, 0); > if (!ret) { > @@ -531,6 +549,7 @@ static int handle_name_to_path(FsContext *ctx, V9fsPath *dir_path, > g_free(fh); > } > close(dirfd); > + g_free(buffer); > return ret; > } > > diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c > index fc93e9e..07d212a 100644 > --- a/hw/9pfs/virtio-9p-local.c > +++ b/hw/9pfs/virtio-9p-local.c > @@ -42,18 +42,18 @@ > > #define VIRTFS_META_DIR ".virtfs_metadata" > > -static const char *local_mapped_attr_path(FsContext *ctx, > - const char *path, char *buffer) > +static char *local_mapped_attr_path(FsContext *ctx, const char *path) > { > char *dir_name; > char *tmp_path = g_strdup(path); > char *base_name = basename(tmp_path); > + char *buffer; > > /* NULL terminate the directory */ > dir_name = tmp_path; > *(base_name - 1) = '\0'; > > - snprintf(buffer, PATH_MAX, "%s/%s/%s/%s", > + buffer = g_strdup_printf("%s/%s/%s/%s", > ctx->fs_root, dir_name, VIRTFS_META_DIR, base_name); > g_free(tmp_path); > return buffer; > @@ -92,10 +92,14 @@ static void local_mapped_file_attr(FsContext *ctx, const char *path, > { > FILE *fp; > char buf[ATTR_MAX]; > - char attr_path[PATH_MAX]; > + char *attr_path; > > - local_mapped_attr_path(ctx, path, attr_path); > + attr_path = local_mapped_attr_path(ctx, path); > + if (!attr_path) { > + return; > + } > fp = local_fopen(attr_path, "r"); > + g_free(attr_path); > if (!fp) { > return; > } > @@ -118,11 +122,17 @@ static void local_mapped_file_attr(FsContext *ctx, const char *path, > static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf) > { > int err; > - char buffer[PATH_MAX]; > + char *buffer; > char *path = fs_path->data; > > - err = lstat(rpath(fs_ctx, path, buffer), stbuf); > + buffer = rpath(fs_ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + err = lstat(buffer, stbuf); > if (err) { > + g_free(buffer); > return err; > } > if (fs_ctx->export_flags & V9FS_SM_MAPPED) { > @@ -131,41 +141,44 @@ static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf) > gid_t tmp_gid; > mode_t tmp_mode; > dev_t tmp_dev; > - if (getxattr(rpath(fs_ctx, path, buffer), "user.virtfs.uid", &tmp_uid, > - sizeof(uid_t)) > 0) { > + if (getxattr(buffer, "user.virtfs.uid", &tmp_uid, sizeof(uid_t)) > 0) { > stbuf->st_uid = tmp_uid; > } > - if (getxattr(rpath(fs_ctx, path, buffer), "user.virtfs.gid", &tmp_gid, > - sizeof(gid_t)) > 0) { > + if (getxattr(buffer, "user.virtfs.gid", &tmp_gid, sizeof(gid_t)) > 0) { > stbuf->st_gid = tmp_gid; > } > - if (getxattr(rpath(fs_ctx, path, buffer), "user.virtfs.mode", > + if (getxattr(buffer, "user.virtfs.mode", > &tmp_mode, sizeof(mode_t)) > 0) { > stbuf->st_mode = tmp_mode; > } > - if (getxattr(rpath(fs_ctx, path, buffer), "user.virtfs.rdev", &tmp_dev, > - sizeof(dev_t)) > 0) { > + if (getxattr(buffer, "user.virtfs.rdev", &tmp_dev, 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); > } > + > + g_free(buffer); > return err; > } > > static int local_create_mapped_attr_dir(FsContext *ctx, const char *path) > { > int err; > - char attr_dir[PATH_MAX]; > + char *attr_dir; > char *tmp_path = g_strdup(path); > > - snprintf(attr_dir, PATH_MAX, "%s/%s/%s", > + attr_dir = g_strdup_printf("%s/%s/%s", > ctx->fs_root, dirname(tmp_path), VIRTFS_META_DIR); > - > + if (!attr_dir) { > + errno = ENOMEM; > + return -1; > + } > err = mkdir(attr_dir, 0700); > if (err < 0 && errno == EEXIST) { > err = 0; > } > + g_free(attr_dir); > g_free(tmp_path); > return err; > } > @@ -176,10 +189,16 @@ static int local_set_mapped_file_attr(FsContext *ctx, > FILE *fp; > int ret = 0; > char buf[ATTR_MAX]; > - char attr_path[PATH_MAX]; > + char *attr_path; > int uid = -1, gid = -1, mode = -1, rdev = -1; > > - fp = local_fopen(local_mapped_attr_path(ctx, path, attr_path), "r"); > + attr_path = local_mapped_attr_path(ctx, path); > + if (!attr_path) { > + errno = ENOMEM; > + return -1; > + } > + fp = local_fopen(attr_path, "r"); > + g_free(attr_path); > if (!fp) { > goto create_map_file; > } > @@ -282,36 +301,52 @@ static int local_set_xattr(const char *path, FsCred *credp) > static int local_post_create_passthrough(FsContext *fs_ctx, const char *path, > FsCred *credp) > { > - char buffer[PATH_MAX]; > + char *buffer; > > - if (lchown(rpath(fs_ctx, path, buffer), credp->fc_uid, > - credp->fc_gid) < 0) { > + buffer = rpath(fs_ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + if (lchown(buffer, 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->export_flags & V9FS_SEC_MASK) != V9FS_SM_NONE) { > - return -1; > + goto err; > } > } > > - if (chmod(rpath(fs_ctx, path, buffer), credp->fc_mode & 07777) < 0) { > - return -1; > + if (chmod(buffer, credp->fc_mode & 07777) < 0) { > + goto err; > } > + > + g_free(buffer); > return 0; > +err: > + g_free(buffer); > + return -1; > } > > static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_path, > char *buf, size_t bufsz) > { > ssize_t tsize = -1; > - char buffer[PATH_MAX]; > + char *buffer; > char *path = fs_path->data; > > + buffer = rpath(fs_ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + > 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 | O_NOFOLLOW); > + fd = open(buffer, O_RDONLY | O_NOFOLLOW); > + g_free(buffer); > if (fd == -1) { > return -1; > } > @@ -322,7 +357,8 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_path, > return tsize; > } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || > (fs_ctx->export_flags & V9FS_SM_NONE)) { > - tsize = readlink(rpath(fs_ctx, path, buffer), buf, bufsz); > + tsize = readlink(buffer, buf, bufsz); > + g_free(buffer); > } > return tsize; > } > @@ -340,20 +376,32 @@ static int local_closedir(FsContext *ctx, V9fsFidOpenState *fs) > static int local_open(FsContext *ctx, V9fsPath *fs_path, > int flags, V9fsFidOpenState *fs) > { > - char buffer[PATH_MAX]; > + char *buffer; > char *path = fs_path->data; > > - fs->fd = open(rpath(ctx, path, buffer), flags | O_NOFOLLOW); > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + fs->fd = open(buffer, flags | O_NOFOLLOW); > + g_free(buffer); > return fs->fd; > } > > static int local_opendir(FsContext *ctx, > V9fsPath *fs_path, V9fsFidOpenState *fs) > { > - char buffer[PATH_MAX]; > + char *buffer; > char *path = fs_path->data; > > - fs->dir = opendir(rpath(ctx, path, buffer)); > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + fs->dir = opendir(buffer); > + g_free(buffer); > if (!fs->dir) { > return -1; > } > @@ -441,18 +489,26 @@ static ssize_t local_pwritev(FsContext *ctx, V9fsFidOpenState *fs, > > static int local_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp) > { > - char buffer[PATH_MAX]; > + char *buffer; > + int ret = -1; > char *path = fs_path->data; > > + buffer = rpath(fs_ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + > if (fs_ctx->export_flags & V9FS_SM_MAPPED) { > - return local_set_xattr(rpath(fs_ctx, path, buffer), credp); > + ret = local_set_xattr(buffer, credp); > } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { > - return local_set_mapped_file_attr(fs_ctx, path, credp); > + ret = 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); > + ret = chmod(buffer, credp->fc_mode); > } > - return -1; > + g_free(buffer); > + return ret; > } > > static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > @@ -462,28 +518,33 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > int err = -1; > int serrno = 0; > V9fsString fullname; > - char buffer[PATH_MAX]; > + char *buffer; > > v9fs_string_init(&fullname); > v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); > path = fullname.data; > > + buffer = rpath(fs_ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + v9fs_string_free(&fullname); > + return -1; > + } > + > /* Determine the security model */ > if (fs_ctx->export_flags & V9FS_SM_MAPPED) { > - err = mknod(rpath(fs_ctx, path, buffer), > - SM_LOCAL_MODE_BITS|S_IFREG, 0); > + err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); > if (err == -1) { > goto out; > } > - err = local_set_xattr(rpath(fs_ctx, path, buffer), credp); > + err = local_set_xattr(buffer, credp); > if (err == -1) { > 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); > + err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); > if (err == -1) { > goto out; > } > @@ -494,8 +555,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > } > } 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, > - credp->fc_rdev); > + err = mknod(buffer, credp->fc_mode, credp->fc_rdev); > if (err == -1) { > goto out; > } > @@ -508,9 +568,10 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > goto out; > > err_end: > - remove(rpath(fs_ctx, path, buffer)); > + remove(buffer); > errno = serrno; > out: > + g_free(buffer); > v9fs_string_free(&fullname); > return err; > } > @@ -522,26 +583,33 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, > int err = -1; > int serrno = 0; > V9fsString fullname; > - char buffer[PATH_MAX]; > + char *buffer; > > v9fs_string_init(&fullname); > v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); > path = fullname.data; > > + buffer = rpath(fs_ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + v9fs_string_free(&fullname); > + return -1; > + } > + > /* Determine the security model */ > if (fs_ctx->export_flags & V9FS_SM_MAPPED) { > - err = mkdir(rpath(fs_ctx, path, buffer), SM_LOCAL_DIR_MODE_BITS); > + err = mkdir(buffer, SM_LOCAL_DIR_MODE_BITS); > if (err == -1) { > goto out; > } > credp->fc_mode = credp->fc_mode|S_IFDIR; > - err = local_set_xattr(rpath(fs_ctx, path, buffer), credp); > + err = local_set_xattr(buffer, credp); > if (err == -1) { > 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); > + err = mkdir(buffer, SM_LOCAL_DIR_MODE_BITS); > if (err == -1) { > goto out; > } > @@ -553,7 +621,7 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, > } > } 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); > + err = mkdir(buffer, credp->fc_mode); > if (err == -1) { > goto out; > } > @@ -566,9 +634,10 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, > goto out; > > err_end: > - remove(rpath(fs_ctx, path, buffer)); > + remove(buffer); > errno = serrno; > out: > + g_free(buffer); > v9fs_string_free(&fullname); > return err; > } > @@ -626,7 +695,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, > int err = -1; > int serrno = 0; > V9fsString fullname; > - char buffer[PATH_MAX]; > + char *buffer; > > /* > * Mark all the open to not follow symlinks > @@ -637,22 +706,29 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, > v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); > path = fullname.data; > > + buffer = rpath(fs_ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + v9fs_string_free(&fullname); > + return -1; > + } > + > /* Determine the security model */ > if (fs_ctx->export_flags & V9FS_SM_MAPPED) { > - fd = open(rpath(fs_ctx, path, buffer), flags, SM_LOCAL_MODE_BITS); > + fd = open(buffer, flags, SM_LOCAL_MODE_BITS); > if (fd == -1) { > err = fd; > goto out; > } > credp->fc_mode = credp->fc_mode|S_IFREG; > /* Set cleint credentials in xattr */ > - err = local_set_xattr(rpath(fs_ctx, path, buffer), credp); > + err = local_set_xattr(buffer, credp); > if (err == -1) { > 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); > + fd = open(buffer, flags, SM_LOCAL_MODE_BITS); > if (fd == -1) { > err = fd; > goto out; > @@ -666,7 +742,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, > } > } 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); > + fd = open(buffer, flags, credp->fc_mode); > if (fd == -1) { > err = fd; > goto out; > @@ -683,9 +759,10 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, > > err_end: > close(fd); > - remove(rpath(fs_ctx, path, buffer)); > + remove(buffer); > errno = serrno; > out: > + g_free(buffer); > v9fs_string_free(&fullname); > return err; > } > @@ -698,19 +775,24 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, > int serrno = 0; > char *newpath; > V9fsString fullname; > - char buffer[PATH_MAX]; > + char *buffer; > > v9fs_string_init(&fullname); > v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); > newpath = fullname.data; > > + buffer = rpath(fs_ctx, newpath); > + if (!buffer) { > + errno = ENOMEM; > + v9fs_string_free(&fullname); > + return -1; > + } > + > /* Determine the security model */ > if (fs_ctx->export_flags & V9FS_SM_MAPPED) { > int fd; > ssize_t oldpath_size, write_size; > - fd = open(rpath(fs_ctx, newpath, buffer), > - O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, > - SM_LOCAL_MODE_BITS); > + fd = open(buffer, O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, SM_LOCAL_MODE_BITS); > if (fd == -1) { > err = fd; > goto out; > @@ -730,7 +812,7 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, > 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, buffer), credp); > + err = local_set_xattr(buffer, credp); > if (err == -1) { > serrno = errno; > goto err_end; > @@ -738,9 +820,7 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, > } 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|O_NOFOLLOW, > - SM_LOCAL_MODE_BITS); > + fd = open(buffer, O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, SM_LOCAL_MODE_BITS); > if (fd == -1) { > err = fd; > goto out; > @@ -767,12 +847,11 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, > } > } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || > (fs_ctx->export_flags & V9FS_SM_NONE)) { > - err = symlink(oldpath, rpath(fs_ctx, newpath, buffer)); > + err = symlink(oldpath, buffer); > if (err) { > goto out; > } > - err = lchown(rpath(fs_ctx, newpath, buffer), credp->fc_uid, > - credp->fc_gid); > + err = lchown(buffer, credp->fc_uid, credp->fc_gid); > if (err == -1) { > /* > * If we fail to change ownership and if we are > @@ -788,9 +867,10 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, > goto out; > > err_end: > - remove(rpath(fs_ctx, newpath, buffer)); > + remove(buffer); > errno = serrno; > out: > + g_free(buffer); > v9fs_string_free(&fullname); > return err; > } > @@ -800,13 +880,26 @@ static int local_link(FsContext *ctx, V9fsPath *oldpath, > { > int ret; > V9fsString newpath; > - char buffer[PATH_MAX], buffer1[PATH_MAX]; > + char *buffer, *buffer1, *buffer0, *buffer01; > > v9fs_string_init(&newpath); > v9fs_string_sprintf(&newpath, "%s/%s", dirpath->data, name); > > - ret = link(rpath(ctx, oldpath->data, buffer), > - rpath(ctx, newpath.data, buffer1)); > + buffer = rpath(ctx, oldpath->data); > + if (!buffer) { > + errno = ENOMEM; > + v9fs_string_free(&newpath); > + return -1; > + } > + buffer1 = rpath(ctx, newpath.data); > + if (!buffer1) { > + errno = ENOMEM; > + g_free(buffer); > + v9fs_string_free(&newpath); > + return -1; > + } > + > + ret = link(buffer, buffer1); > > /* now link the virtfs_metadata files */ > if (!ret && (ctx->export_flags & V9FS_SM_MAPPED_FILE)) { > @@ -815,81 +908,156 @@ static int local_link(FsContext *ctx, V9fsPath *oldpath, > if (ret < 0) { > goto err_out; > } > - ret = link(local_mapped_attr_path(ctx, oldpath->data, buffer), > - local_mapped_attr_path(ctx, newpath.data, buffer1)); > + buffer0 = local_mapped_attr_path(ctx, oldpath->data); > + if (!buffer0) { > + errno = ENOMEM; > + ret = -1; > + goto err_out; > + } > + buffer01 = local_mapped_attr_path(ctx, newpath.data); > + if (!buffer01) { > + errno = ENOMEM; > + g_free(buffer01); > + ret = -1; > + goto err_out; > + } > + ret = link(buffer0, buffer01); > + g_free(buffer0); > + g_free(buffer01); > if (ret < 0 && errno != ENOENT) { > goto err_out; > } > } > err_out: > + g_free(buffer); > + g_free(buffer1); > v9fs_string_free(&newpath); > return ret; > } > > static int local_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size) > { > - char buffer[PATH_MAX]; > + char *buffer; > + int ret; > char *path = fs_path->data; > > - return truncate(rpath(ctx, path, buffer), size); > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = truncate(buffer, size); > + g_free(buffer); > + return ret; > } > > static int local_rename(FsContext *ctx, const char *oldpath, > const char *newpath) > { > int err; > - char buffer[PATH_MAX], buffer1[PATH_MAX]; > + char *buffer, *buffer1, *buffer0, *buffer01; > + > + buffer = rpath(ctx, oldpath); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + buffer1 = rpath(ctx, newpath); > + if (!buffer1) { > + errno = ENOMEM; > + g_free(buffer); > + return -1; > + } > > if (ctx->export_flags & V9FS_SM_MAPPED_FILE) { > err = local_create_mapped_attr_dir(ctx, newpath); > if (err < 0) { > - return err; > + goto ret_err; > } > /* rename the .virtfs_metadata files */ > - err = rename(local_mapped_attr_path(ctx, oldpath, buffer), > - local_mapped_attr_path(ctx, newpath, buffer1)); > + buffer0 = local_mapped_attr_path(ctx, oldpath); > + if (!buffer0) { > + errno = ENOMEM; > + err = -1; > + goto ret_err; > + } > + buffer01 = local_mapped_attr_path(ctx, newpath); > + if (!buffer01) { > + errno = ENOMEM; > + g_free(buffer01); > + err = -1; > + goto ret_err; > + } > + err = rename(buffer0, buffer01); > + g_free(buffer0); > + g_free(buffer01); > if (err < 0 && errno != ENOENT) { > - return err; > + goto ret_err; > } > } > - return rename(rpath(ctx, oldpath, buffer), rpath(ctx, newpath, buffer1)); > + err = rename(buffer, buffer1); > +ret_err: > + g_free(buffer1); > + g_free(buffer); > + return err; > } > > static int local_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp) > { > - char buffer[PATH_MAX]; > + char *buffer; > + int ret = -1; > char *path = fs_path->data; > > + buffer = rpath(fs_ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > if ((credp->fc_uid == -1 && credp->fc_gid == -1) || > (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || > (fs_ctx->export_flags & V9FS_SM_NONE)) { > - return lchown(rpath(fs_ctx, path, buffer), > - credp->fc_uid, credp->fc_gid); > + ret = lchown(buffer, 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); > + ret = local_set_xattr(buffer, credp); > } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { > - return local_set_mapped_file_attr(fs_ctx, path, credp); > + ret = local_set_mapped_file_attr(fs_ctx, path, credp); > } > - return -1; > + g_free(buffer); > + return ret; > } > > static int local_utimensat(FsContext *s, V9fsPath *fs_path, > const struct timespec *buf) > { > - char buffer[PATH_MAX]; > + char *buffer; > + int ret; > char *path = fs_path->data; > > - return qemu_utimens(rpath(s, path, buffer), buf); > + buffer = rpath(s, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = qemu_utimens(buffer, buf); > + g_free(buffer); > + return ret; > } > > static int local_remove(FsContext *ctx, const char *path) > { > int err; > struct stat stbuf; > - char buffer[PATH_MAX]; > + char *buffer; > + char *buffer1; > + > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > > if (ctx->export_flags & V9FS_SM_MAPPED_FILE) { > - err = lstat(rpath(ctx, path, buffer), &stbuf); > + err = lstat(buffer, &stbuf); > if (err) { > goto err_out; > } > @@ -898,8 +1066,15 @@ static int local_remove(FsContext *ctx, const char *path) > * directory > */ > if (S_ISDIR(stbuf.st_mode)) { > - sprintf(buffer, "%s/%s/%s", ctx->fs_root, path, VIRTFS_META_DIR); > - err = remove(buffer); > + buffer1 = g_strdup_printf("%s/%s/%s", ctx->fs_root, > + path, VIRTFS_META_DIR); > + if (!buffer1) { > + errno = ENOMEM; > + err = -1; > + goto err_out; > + } > + err = remove(buffer1); > + g_free(buffer1); > if (err < 0 && errno != ENOENT) { > /* > * We didn't had the .virtfs_metadata file. May be file created > @@ -912,7 +1087,14 @@ static int local_remove(FsContext *ctx, const char *path) > * Now remove the name from parent directory > * .virtfs_metadata directory > */ > - err = remove(local_mapped_attr_path(ctx, path, buffer)); > + buffer1 = local_mapped_attr_path(ctx, path); > + if (!buffer1) { > + errno = ENOMEM; > + err = -1; > + goto err_out; > + } > + err = remove(buffer1); > + g_free(buffer1); > if (err < 0 && errno != ENOENT) { > /* > * We didn't had the .virtfs_metadata file. May be file created > @@ -921,8 +1103,10 @@ static int local_remove(FsContext *ctx, const char *path) > goto err_out; > } > } > - return remove(rpath(ctx, path, buffer)); > + > + err = remove(buffer); > err_out: > + g_free(buffer); > return err; > } > > @@ -946,10 +1130,18 @@ static int local_fsync(FsContext *ctx, int fid_type, > > static int local_statfs(FsContext *s, V9fsPath *fs_path, struct statfs *stbuf) > { > - char buffer[PATH_MAX]; > + char *buffer; > + int ret; > char *path = fs_path->data; > > - return statfs(rpath(s, path, buffer), stbuf); > + buffer = rpath(s, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = statfs(buffer, stbuf); > + g_free(buffer); > + return ret; > } > > static ssize_t local_lgetxattr(FsContext *ctx, V9fsPath *fs_path, > @@ -1022,20 +1214,39 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir, > { > int ret; > V9fsString fullname; > - char buffer[PATH_MAX]; > + char *buffer0 = NULL, *buffer1 = NULL, *buffer2 = NULL; > > v9fs_string_init(&fullname); > - > v9fs_string_sprintf(&fullname, "%s/%s", dir->data, name); > + > + buffer0 = rpath(ctx, fullname.data); > + if (!buffer0) { > + errno = ENOMEM; > + ret = -1; > + goto err_out; > + } > + > if (ctx->export_flags & V9FS_SM_MAPPED_FILE) { > + buffer1 = local_mapped_attr_path(ctx, fullname.data); > + if (!buffer1) { > + errno = ENOMEM; > + ret = -1; > + goto err_out; > + } > + > if (flags == AT_REMOVEDIR) { > /* > * If directory remove .virtfs_metadata contained in the > * directory > */ > - sprintf(buffer, "%s/%s/%s", ctx->fs_root, > + buffer2 = g_strdup_printf("%s/%s/%s", ctx->fs_root, > fullname.data, VIRTFS_META_DIR); > - ret = remove(buffer); > + if (!buffer2) { > + errno = ENOMEM; > + ret = -1; > + goto err_out; > + } > + ret = remove(buffer2); > if (ret < 0 && errno != ENOENT) { > /* > * We didn't had the .virtfs_metadata file. May be file created > @@ -1048,7 +1259,7 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir, > * Now remove the name from parent directory > * .virtfs_metadata directory. > */ > - ret = remove(local_mapped_attr_path(ctx, fullname.data, buffer)); > + ret = remove(buffer1); > if (ret < 0 && errno != ENOENT) { > /* > * We didn't had the .virtfs_metadata file. May be file created > @@ -1058,10 +1269,18 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir, > } > } > /* Remove the name finally */ > - ret = remove(rpath(ctx, fullname.data, buffer)); > - v9fs_string_free(&fullname); > - > + ret = remove(buffer0); > err_out: > + if (buffer0) { > + g_free(buffer0); > + } > + if (buffer1) { > + g_free(buffer1); > + } > + if (buffer2) { > + g_free(buffer2); > + } > + v9fs_string_free(&fullname); > return ret; > } > > diff --git a/hw/9pfs/virtio-9p-posix-acl.c b/hw/9pfs/virtio-9p-posix-acl.c > index 339c5ec..ffae353 100644 > --- a/hw/9pfs/virtio-9p-posix-acl.c > +++ b/hw/9pfs/virtio-9p-posix-acl.c > @@ -26,8 +26,17 @@ > static ssize_t mp_pacl_getxattr(FsContext *ctx, const char *path, > const char *name, void *value, size_t size) > { > - char buffer[PATH_MAX]; > - return lgetxattr(rpath(ctx, path, buffer), MAP_ACL_ACCESS, value, size); > + char *buffer; > + ssize_t ret; > + > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lgetxattr(buffer, MAP_ACL_ACCESS, value, size); > + g_free(buffer); > + return ret; > } > > static ssize_t mp_pacl_listxattr(FsContext *ctx, const char *path, > @@ -52,17 +61,31 @@ static ssize_t mp_pacl_listxattr(FsContext *ctx, const char *path, > static int mp_pacl_setxattr(FsContext *ctx, const char *path, const char *name, > void *value, size_t size, int flags) > { > - char buffer[PATH_MAX]; > - return lsetxattr(rpath(ctx, path, buffer), MAP_ACL_ACCESS, value, > - size, flags); > + char *buffer; > + int ret; > + > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lsetxattr(buffer, MAP_ACL_ACCESS, value, size, flags); > + g_free(buffer); > + return ret; > } > > static int mp_pacl_removexattr(FsContext *ctx, > const char *path, const char *name) > { > int ret; > - char buffer[PATH_MAX]; > - ret = lremovexattr(rpath(ctx, path, buffer), MAP_ACL_ACCESS); > + char *buffer; > + > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lremovexattr(buffer, MAP_ACL_ACCESS); > if (ret == -1 && errno == ENODATA) { > /* > * We don't get ENODATA error when trying to remove a > @@ -72,14 +95,24 @@ static int mp_pacl_removexattr(FsContext *ctx, > errno = 0; > ret = 0; > } > + g_free(buffer); > return ret; > } > > static ssize_t mp_dacl_getxattr(FsContext *ctx, const char *path, > const char *name, void *value, size_t size) > { > - char buffer[PATH_MAX]; > - return lgetxattr(rpath(ctx, path, buffer), MAP_ACL_DEFAULT, value, size); > + char *buffer; > + ssize_t ret; > + > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lgetxattr(buffer, MAP_ACL_DEFAULT, value, size); > + g_free(buffer); > + return ret; > } > > static ssize_t mp_dacl_listxattr(FsContext *ctx, const char *path, > @@ -104,17 +137,31 @@ static ssize_t mp_dacl_listxattr(FsContext *ctx, const char *path, > static int mp_dacl_setxattr(FsContext *ctx, const char *path, const char *name, > void *value, size_t size, int flags) > { > - char buffer[PATH_MAX]; > - return lsetxattr(rpath(ctx, path, buffer), MAP_ACL_DEFAULT, value, > - size, flags); > + char *buffer; > + int ret; > + > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lsetxattr(buffer, MAP_ACL_DEFAULT, value, size, flags); > + g_free(buffer); > + return ret; > } > > static int mp_dacl_removexattr(FsContext *ctx, > const char *path, const char *name) > { > int ret; > - char buffer[PATH_MAX]; > - ret = lremovexattr(rpath(ctx, path, buffer), MAP_ACL_DEFAULT); > + char *buffer; > + > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lremovexattr(buffer, MAP_ACL_DEFAULT); > if (ret == -1 && errno == ENODATA) { > /* > * We don't get ENODATA error when trying to remove a > @@ -124,6 +171,7 @@ static int mp_dacl_removexattr(FsContext *ctx, > errno = 0; > ret = 0; > } > + g_free(buffer); > return ret; > } > > diff --git a/hw/9pfs/virtio-9p-xattr-user.c b/hw/9pfs/virtio-9p-xattr-user.c > index e0c92eb..6f82710 100644 > --- a/hw/9pfs/virtio-9p-xattr-user.c > +++ b/hw/9pfs/virtio-9p-xattr-user.c > @@ -21,7 +21,8 @@ > static ssize_t mp_user_getxattr(FsContext *ctx, const char *path, > const char *name, void *value, size_t size) > { > - char buffer[PATH_MAX]; > + char *buffer; > + ssize_t ret; > if (strncmp(name, "user.virtfs.", 12) == 0) { > /* > * Don't allow fetch of user.virtfs namesapce > @@ -30,7 +31,14 @@ static ssize_t mp_user_getxattr(FsContext *ctx, const char *path, > errno = ENOATTR; > return -1; > } > - return lgetxattr(rpath(ctx, path, buffer), name, value, size); > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lgetxattr(buffer, name, value, size); > + g_free(buffer); > + return ret; > } > > static ssize_t mp_user_listxattr(FsContext *ctx, const char *path, > @@ -69,7 +77,8 @@ static ssize_t mp_user_listxattr(FsContext *ctx, const char *path, > static int mp_user_setxattr(FsContext *ctx, const char *path, const char *name, > void *value, size_t size, int flags) > { > - char buffer[PATH_MAX]; > + char *buffer; > + int ret; > if (strncmp(name, "user.virtfs.", 12) == 0) { > /* > * Don't allow fetch of user.virtfs namesapce > @@ -78,13 +87,21 @@ static int mp_user_setxattr(FsContext *ctx, const char *path, const char *name, > errno = EACCES; > return -1; > } > - return lsetxattr(rpath(ctx, path, buffer), name, value, size, flags); > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lsetxattr(buffer, name, value, size, flags); > + g_free(buffer); > + return ret; > } > > static int mp_user_removexattr(FsContext *ctx, > const char *path, const char *name) > { > - char buffer[PATH_MAX]; > + char *buffer; > + int ret; > if (strncmp(name, "user.virtfs.", 12) == 0) { > /* > * Don't allow fetch of user.virtfs namesapce > @@ -93,7 +110,14 @@ static int mp_user_removexattr(FsContext *ctx, > errno = EACCES; > return -1; > } > - return lremovexattr(rpath(ctx, path, buffer), name); > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lremovexattr(buffer, name); > + g_free(buffer); > + return ret; > } > > XattrOperations mapped_user_xattr = { > diff --git a/hw/9pfs/virtio-9p-xattr.c b/hw/9pfs/virtio-9p-xattr.c > index 3fae557..1f5008a 100644 > --- a/hw/9pfs/virtio-9p-xattr.c > +++ b/hw/9pfs/virtio-9p-xattr.c > @@ -67,21 +67,28 @@ ssize_t v9fs_list_xattr(FsContext *ctx, const char *path, > void *value, size_t vsize) > { > ssize_t size = 0; > - char buffer[PATH_MAX]; > + char *buffer; > void *ovalue = value; > XattrOperations *xops; > char *orig_value, *orig_value_start; > ssize_t xattr_len, parsed_len = 0, attr_len; > > /* Get the actual len */ > - xattr_len = llistxattr(rpath(ctx, path, buffer), value, 0); > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + xattr_len = llistxattr(buffer, value, 0); > if (xattr_len <= 0) { > + g_free(buffer); > return xattr_len; > } > > /* Now fetch the xattr and find the actual size */ > orig_value = g_malloc(xattr_len); > - xattr_len = llistxattr(rpath(ctx, path, buffer), orig_value, xattr_len); > + xattr_len = llistxattr(buffer, orig_value, xattr_len); > + g_free(buffer); > > /* store the orig pointer */ > orig_value_start = orig_value; > diff --git a/hw/9pfs/virtio-9p-xattr.h b/hw/9pfs/virtio-9p-xattr.h > index 41cc6cb..683d37b 100644 > --- a/hw/9pfs/virtio-9p-xattr.h > +++ b/hw/9pfs/virtio-9p-xattr.h > @@ -54,23 +54,50 @@ ssize_t pt_listxattr(FsContext *ctx, const char *path, char *name, void *value, > static inline ssize_t pt_getxattr(FsContext *ctx, const char *path, > const char *name, void *value, size_t size) > { > - char buffer[PATH_MAX]; > - return lgetxattr(rpath(ctx, path, buffer), name, value, size); > + char *buffer; > + ssize_t ret; > + > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lgetxattr(buffer, name, value, size); > + g_free(buffer); > + return ret; > } > > static inline int pt_setxattr(FsContext *ctx, const char *path, > const char *name, void *value, > size_t size, int flags) > { > - char buffer[PATH_MAX]; > - return lsetxattr(rpath(ctx, path, buffer), name, value, size, flags); > + char *buffer; > + int ret; > + > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lsetxattr(buffer, name, value, size, flags); > + g_free(buffer); > + return ret; > } > > static inline int pt_removexattr(FsContext *ctx, > const char *path, const char *name) > { > - char buffer[PATH_MAX]; > - return lremovexattr(rpath(ctx, path, buffer), name); > + char *buffer; > + int ret; > + > + buffer = rpath(ctx, path); > + if (!buffer) { > + errno = ENOMEM; > + return -1; > + } > + ret = lremovexattr(buffer, name); > + g_free(buffer); > + return ret; > } > > static inline ssize_t notsup_getxattr(FsContext *ctx, const char *path, > diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h > index 1d6eedb..2c3603a 100644 > --- a/hw/9pfs/virtio-9p.h > +++ b/hw/9pfs/virtio-9p.h > @@ -6,6 +6,7 @@ > #include <sys/time.h> > #include <utime.h> > #include <sys/resource.h> > +#include <glib.h> > #include "hw/virtio/virtio.h" > #include "fsdev/file-op-9p.h" > #include "fsdev/virtio-9p-marshal.h" > @@ -112,10 +113,9 @@ enum p9_proto_version { > > #define FID_REFERENCED 0x1 > #define FID_NON_RECLAIMABLE 0x2 > -static inline const char *rpath(FsContext *ctx, const char *path, char *buffer) > +static inline char *rpath(FsContext *ctx, const char *path) > { > - snprintf(buffer, PATH_MAX, "%s/%s", ctx->fs_root, path); > - return buffer; > + return g_strdup_printf("%s/%s", ctx->fs_root, path); > } > > /* >
Chen Gang <gang.chen.5i5j@gmail.com> writes: > When path is truncated by PATH_MAX limitation, it causes QEMU to access > incorrect file. So use original full path instead of PATH_MAX within > 9pfs (need check/process ENOMEM for related memory allocation). > > Also find/fix several another related issues when failure occurs. > > - check 'fh' in handle_name_to_path(). > > - need call v9fs_string_free() at 'err_out:' in local_unlinkat(). > > - sprintf() will cause memory overflow in "virtio-9p-local.c" Sound like distinct bugs. Have you considered fixing them in separate patches for easier review and clearer commit messages? > The related test: > > - Environments (for qemu-devel): > > - Host is under fedora17 desktop with ext4fs: > > qemu-system-x86_64 -hda test.img -m 1024 \ > -net nic,vlan=4,model=virtio,macaddr=00:16:35:AF:94:04 \ > -net tap,vlan=4,ifname=tap4,script=no,downscript=no \ > -device virtio-9p-pci,id=fs0,fsdev=fsdev0,mount_tag=hostshare \ > -fsdev local,security_model=passthrough,id=fsdev0,\ > path=/upstream/vm/data/share/1234567890abcdefghijklmnopqrstuvwxyz\ > ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890acdefghijklmnopqrstuvwxyz\ > ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890/111111111111111111111111111\ > 1111111111111111111111111111111111111111111111111111222222222222\ > 2222222222222222222222222222222222222222222222222222222222222222\ > 2222222222222222222222222222222222233333333333333333333333333333\ > 3333333333333333333333333333333333 > > - Guest is ubuntu12 server with 9pfs. > > mount -t 9p -o trans=virtio,version=9p2000.L hostshare /share > > - Limitations: > > full path limitation is PATH_MAX (4096B include nul) under Linux. > file/dir node name maximized length is 256 (include nul) under ext4. > > - Special test: > > Under host, modify the file: "/upstream/vm/data/share/1234567890abcdefg\ [...] > ppppppppppppppppppppppppppppppppppppppp/test1234567890file.log" > (need enter dir firstly, then modify file, or can not open it). > > Under guest, still allow modify "test1234567890file.log" (will generate > "test123456" file with contents). > > After apply this patch, can not open "test1234567890file.log" under guest > (permission denied). > > > - Common test: > > All are still OK after apply this path. > > "mkdir -p", "create/open file/dir", "modify file/dir", "rm file/dir". > change various mount point paths under host and/or guest. > > > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> > --- > hw/9pfs/cofs.c | 22 ++- > hw/9pfs/virtio-9p-handle.c | 25 ++- > hw/9pfs/virtio-9p-local.c | 433 +++++++++++++++++++++++++++++++---------- > hw/9pfs/virtio-9p-posix-acl.c | 76 ++++++-- > hw/9pfs/virtio-9p-xattr-user.c | 36 +++- > hw/9pfs/virtio-9p-xattr.c | 13 +- > hw/9pfs/virtio-9p-xattr.h | 39 +++- > hw/9pfs/virtio-9p.h | 6 +- > 8 files changed, 504 insertions(+), 146 deletions(-) > > diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c > index 3891050..ba69965 100644 > --- a/hw/9pfs/cofs.c > +++ b/hw/9pfs/cofs.c > @@ -20,18 +20,31 @@ > int v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf) > { > int err; > - ssize_t len; > + ssize_t len, maxlen = PATH_MAX; > V9fsState *s = pdu->s; > > if (v9fs_request_cancelled(pdu)) { > return -EINTR; > } > - buf->data = g_malloc(PATH_MAX); > + buf->data = g_malloc(maxlen); > + if (!buf->data) { Can't happen, because g_malloc() returns NULL only when its argument is zero. Many more instances below, also with other memory allocation functions, such as g_strdup_printf(). Please clean up and resubmit. > + return -ENOMEM; > + } [...]
Firstly, thank you very much for your reviewing. :-) On 02/24/2014 05:22 PM, Markus Armbruster wrote: > Chen Gang <gang.chen.5i5j@gmail.com> writes: > >> When path is truncated by PATH_MAX limitation, it causes QEMU to access >> incorrect file. So use original full path instead of PATH_MAX within >> 9pfs (need check/process ENOMEM for related memory allocation). >> >> Also find/fix several another related issues when failure occurs. >> >> - check 'fh' in handle_name_to_path(). >> As you said at the bottom, it is not an issue, need be skipped. >> - need call v9fs_string_free() at 'err_out:' in local_unlinkat(). >> >> - sprintf() will cause memory overflow in "virtio-9p-local.c" > > Sound like distinct bugs. Have you considered fixing them in separate > patches for easier review and clearer commit messages? > OK, I will/should separate them, but I have to send them (the 2 patches for 2 issues above) firstly, then send this patch (total 3 patches). > [...] > Can't happen, because g_malloc() returns NULL only when its argument is > zero. Many more instances below, also with other memory allocation > functions, such as g_strdup_printf(). Please clean up and resubmit. > OK, thank you, I will/should clean up and resubmit. Excuse me, I have no enough time resources during work day, so I will/should send the patches within week end (2014-03-02). If we can not bear the time point, please help send the patches for me. Thanks.
Gang Chen <gang.chen.5i5j@gmail.com> writes: > Firstly, thank you very much for your reviewing. :-) > > On 02/24/2014 05:22 PM, Markus Armbruster wrote: >> Chen Gang <gang.chen.5i5j@gmail.com> writes: >> >>> When path is truncated by PATH_MAX limitation, it causes QEMU to access >>> incorrect file. So use original full path instead of PATH_MAX within >>> 9pfs (need check/process ENOMEM for related memory allocation). >>> >>> Also find/fix several another related issues when failure occurs. >>> >>> - check 'fh' in handle_name_to_path(). >>> > > As you said at the bottom, it is not an issue, need be skipped. > >>> - need call v9fs_string_free() at 'err_out:' in local_unlinkat(). >>> >>> - sprintf() will cause memory overflow in "virtio-9p-local.c" >> >> Sound like distinct bugs. Have you considered fixing them in separate >> patches for easier review and clearer commit messages? >> > > OK, I will/should separate them, but I have to send them (the 2 patches > for 2 issues above) firstly, then send this patch (total 3 patches). > >> > [...] >> Can't happen, because g_malloc() returns NULL only when its argument is >> zero. Many more instances below, also with other memory allocation >> functions, such as g_strdup_printf(). Please clean up and resubmit. >> > > OK, thank you, I will/should clean up and resubmit. > > Excuse me, I have no enough time resources during work day, so I > will/should send the patches within week end (2014-03-02). If we can not > bear the time point, please help send the patches for me. Next weekend should be fine. Happy hacking!
On 02/24/2014 08:52 PM, Markus Armbruster wrote: > Gang Chen <gang.chen.5i5j@gmail.com> writes: >> Excuse me, I have no enough time resources during work day, so I >> will/should send the patches within week end (2014-03-02). If we can not >> bear the time point, please help send the patches for me. > > Next weekend should be fine. Happy hacking! > OK, thank you. Thanks
Patch 1/3: move v9fs_string_free() to below "err_out:"
Patch 2/3: use snprintf() instead of sprintf()
(which will be replaced of by Path 3/3)
Patch 3/3: use g_strdup_printf() instead of PATH_MAX limitation
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
hw/9pfs/cofs.c | 15 ++-
hw/9pfs/virtio-9p-handle.c | 9 +-
hw/9pfs/virtio-9p-local.c | 286 +++++++++++++++++++++++++++--------------
hw/9pfs/virtio-9p-posix-acl.c | 52 ++++++--
hw/9pfs/virtio-9p-xattr-user.c | 27 +++-
hw/9pfs/virtio-9p-xattr.c | 9 +-
hw/9pfs/virtio-9p-xattr.h | 27 +++-
hw/9pfs/virtio-9p.h | 6 +-
8 files changed, 293 insertions(+), 138 deletions(-)
On 03/01/2014 10:33 AM, Chen Gang wrote: > Patch 1/3: move v9fs_string_free() to below "err_out:" > > Patch 2/3: use snprintf() instead of sprintf() > (which will be replaced of by Path 3/3) > > Patch 3/3: use g_strdup_printf() instead of PATH_MAX limitation By sending this series as a deeply nested reply (14 References: listings), it's harder to note that it is an independent series. It's best to send your 0/N cover letter as a new top-level thread, rather than burying it an existing thread.
On 03/04/2014 01:43 AM, Eric Blake wrote: > On 03/01/2014 10:33 AM, Chen Gang wrote: >> Patch 1/3: move v9fs_string_free() to below "err_out:" >> >> Patch 2/3: use snprintf() instead of sprintf() >> (which will be replaced of by Path 3/3) >> >> Patch 3/3: use g_strdup_printf() instead of PATH_MAX limitation > > By sending this series as a deeply nested reply (14 References: > listings), it's harder to note that it is an independent series. It's > best to send your 0/N cover letter as a new top-level thread, rather > than burying it an existing thread. > OK, thanks. My current 0/N patch is almost useless, it really should/need provide some more useful information. Next time, when I send a new series, I shall/need reference the 0/N cover letter of another members' series, firstly. Thanks
diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c index 3891050..ba69965 100644 --- a/hw/9pfs/cofs.c +++ b/hw/9pfs/cofs.c @@ -20,18 +20,31 @@ int v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf) { int err; - ssize_t len; + ssize_t len, maxlen = PATH_MAX; V9fsState *s = pdu->s; if (v9fs_request_cancelled(pdu)) { return -EINTR; } - buf->data = g_malloc(PATH_MAX); + buf->data = g_malloc(maxlen); + if (!buf->data) { + return -ENOMEM; + } v9fs_path_read_lock(s); v9fs_co_run_in_worker( - { + while (1) { len = s->ops->readlink(&s->ctx, path, - buf->data, PATH_MAX - 1); + buf->data, maxlen - 1); + if (len == maxlen - 1) { + g_free(buf->data); + maxlen *= 2; + buf->data = g_malloc(maxlen); + if (!buf->data) { + err = -errno; + break; + } + continue; + } if (len > -1) { buf->size = len; buf->data[len] = 0; @@ -39,6 +52,7 @@ int v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf) } else { err = -errno; } + break; }); v9fs_path_unlock(s); if (err) { diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c index fe8e0ed..fb19fb0 100644 --- a/hw/9pfs/virtio-9p-handle.c +++ b/hw/9pfs/virtio-9p-handle.c @@ -498,7 +498,7 @@ static int handle_lremovexattr(FsContext *ctx, V9fsPath *fs_path, static int handle_name_to_path(FsContext *ctx, V9fsPath *dir_path, const char *name, V9fsPath *target) { - char buffer[PATH_MAX]; + char *buffer; struct file_handle *fh; int dirfd, ret, mnt_id; struct handle_data *data = (struct handle_data *)ctx->private; @@ -513,15 +513,33 @@ static int handle_name_to_path(FsContext *ctx, V9fsPath *dir_path, dirfd = open_by_handle(data->mountfd, dir_path->data, O_PATH); } else { /* relative to export root */ - dirfd = open(rpath(ctx, ".", buffer), O_DIRECTORY); + buffer = rpath(ctx, "."); + if (!buffer) { + errno = ENOMEM; + return -1; + } + dirfd = open(buffer, O_DIRECTORY); + g_free(buffer); } if (dirfd < 0) { return dirfd; } + fh = g_malloc(sizeof(struct file_handle) + data->handle_bytes); + if (!fh) { + close(dirfd); + errno = ENOMEM; + return -1; + } fh->handle_bytes = data->handle_bytes; /* add a "./" at the beginning of the path */ - snprintf(buffer, PATH_MAX, "./%s", name); + buffer = g_strdup_printf("./%s", name); + if (!buffer) { + g_free(fh); + close(dirfd); + errno = ENOMEM; + return -1; + } /* flag = 0 imply don't follow symlink */ ret = name_to_handle(dirfd, buffer, fh, &mnt_id, 0); if (!ret) { @@ -531,6 +549,7 @@ static int handle_name_to_path(FsContext *ctx, V9fsPath *dir_path, g_free(fh); } close(dirfd); + g_free(buffer); return ret; } diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index fc93e9e..07d212a 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -42,18 +42,18 @@ #define VIRTFS_META_DIR ".virtfs_metadata" -static const char *local_mapped_attr_path(FsContext *ctx, - const char *path, char *buffer) +static char *local_mapped_attr_path(FsContext *ctx, const char *path) { char *dir_name; char *tmp_path = g_strdup(path); char *base_name = basename(tmp_path); + char *buffer; /* NULL terminate the directory */ dir_name = tmp_path; *(base_name - 1) = '\0'; - snprintf(buffer, PATH_MAX, "%s/%s/%s/%s", + buffer = g_strdup_printf("%s/%s/%s/%s", ctx->fs_root, dir_name, VIRTFS_META_DIR, base_name); g_free(tmp_path); return buffer; @@ -92,10 +92,14 @@ static void local_mapped_file_attr(FsContext *ctx, const char *path, { FILE *fp; char buf[ATTR_MAX]; - char attr_path[PATH_MAX]; + char *attr_path; - local_mapped_attr_path(ctx, path, attr_path); + attr_path = local_mapped_attr_path(ctx, path); + if (!attr_path) { + return; + } fp = local_fopen(attr_path, "r"); + g_free(attr_path); if (!fp) { return; } @@ -118,11 +122,17 @@ static void local_mapped_file_attr(FsContext *ctx, const char *path, static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf) { int err; - char buffer[PATH_MAX]; + char *buffer; char *path = fs_path->data; - err = lstat(rpath(fs_ctx, path, buffer), stbuf); + buffer = rpath(fs_ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + err = lstat(buffer, stbuf); if (err) { + g_free(buffer); return err; } if (fs_ctx->export_flags & V9FS_SM_MAPPED) { @@ -131,41 +141,44 @@ static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf) gid_t tmp_gid; mode_t tmp_mode; dev_t tmp_dev; - if (getxattr(rpath(fs_ctx, path, buffer), "user.virtfs.uid", &tmp_uid, - sizeof(uid_t)) > 0) { + if (getxattr(buffer, "user.virtfs.uid", &tmp_uid, sizeof(uid_t)) > 0) { stbuf->st_uid = tmp_uid; } - if (getxattr(rpath(fs_ctx, path, buffer), "user.virtfs.gid", &tmp_gid, - sizeof(gid_t)) > 0) { + if (getxattr(buffer, "user.virtfs.gid", &tmp_gid, sizeof(gid_t)) > 0) { stbuf->st_gid = tmp_gid; } - if (getxattr(rpath(fs_ctx, path, buffer), "user.virtfs.mode", + if (getxattr(buffer, "user.virtfs.mode", &tmp_mode, sizeof(mode_t)) > 0) { stbuf->st_mode = tmp_mode; } - if (getxattr(rpath(fs_ctx, path, buffer), "user.virtfs.rdev", &tmp_dev, - sizeof(dev_t)) > 0) { + if (getxattr(buffer, "user.virtfs.rdev", &tmp_dev, 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); } + + g_free(buffer); return err; } static int local_create_mapped_attr_dir(FsContext *ctx, const char *path) { int err; - char attr_dir[PATH_MAX]; + char *attr_dir; char *tmp_path = g_strdup(path); - snprintf(attr_dir, PATH_MAX, "%s/%s/%s", + attr_dir = g_strdup_printf("%s/%s/%s", ctx->fs_root, dirname(tmp_path), VIRTFS_META_DIR); - + if (!attr_dir) { + errno = ENOMEM; + return -1; + } err = mkdir(attr_dir, 0700); if (err < 0 && errno == EEXIST) { err = 0; } + g_free(attr_dir); g_free(tmp_path); return err; } @@ -176,10 +189,16 @@ static int local_set_mapped_file_attr(FsContext *ctx, FILE *fp; int ret = 0; char buf[ATTR_MAX]; - char attr_path[PATH_MAX]; + char *attr_path; int uid = -1, gid = -1, mode = -1, rdev = -1; - fp = local_fopen(local_mapped_attr_path(ctx, path, attr_path), "r"); + attr_path = local_mapped_attr_path(ctx, path); + if (!attr_path) { + errno = ENOMEM; + return -1; + } + fp = local_fopen(attr_path, "r"); + g_free(attr_path); if (!fp) { goto create_map_file; } @@ -282,36 +301,52 @@ static int local_set_xattr(const char *path, FsCred *credp) static int local_post_create_passthrough(FsContext *fs_ctx, const char *path, FsCred *credp) { - char buffer[PATH_MAX]; + char *buffer; - if (lchown(rpath(fs_ctx, path, buffer), credp->fc_uid, - credp->fc_gid) < 0) { + buffer = rpath(fs_ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + if (lchown(buffer, 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->export_flags & V9FS_SEC_MASK) != V9FS_SM_NONE) { - return -1; + goto err; } } - if (chmod(rpath(fs_ctx, path, buffer), credp->fc_mode & 07777) < 0) { - return -1; + if (chmod(buffer, credp->fc_mode & 07777) < 0) { + goto err; } + + g_free(buffer); return 0; +err: + g_free(buffer); + return -1; } static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_path, char *buf, size_t bufsz) { ssize_t tsize = -1; - char buffer[PATH_MAX]; + char *buffer; char *path = fs_path->data; + buffer = rpath(fs_ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + 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 | O_NOFOLLOW); + fd = open(buffer, O_RDONLY | O_NOFOLLOW); + g_free(buffer); if (fd == -1) { return -1; } @@ -322,7 +357,8 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_path, return tsize; } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || (fs_ctx->export_flags & V9FS_SM_NONE)) { - tsize = readlink(rpath(fs_ctx, path, buffer), buf, bufsz); + tsize = readlink(buffer, buf, bufsz); + g_free(buffer); } return tsize; } @@ -340,20 +376,32 @@ static int local_closedir(FsContext *ctx, V9fsFidOpenState *fs) static int local_open(FsContext *ctx, V9fsPath *fs_path, int flags, V9fsFidOpenState *fs) { - char buffer[PATH_MAX]; + char *buffer; char *path = fs_path->data; - fs->fd = open(rpath(ctx, path, buffer), flags | O_NOFOLLOW); + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + fs->fd = open(buffer, flags | O_NOFOLLOW); + g_free(buffer); return fs->fd; } static int local_opendir(FsContext *ctx, V9fsPath *fs_path, V9fsFidOpenState *fs) { - char buffer[PATH_MAX]; + char *buffer; char *path = fs_path->data; - fs->dir = opendir(rpath(ctx, path, buffer)); + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + fs->dir = opendir(buffer); + g_free(buffer); if (!fs->dir) { return -1; } @@ -441,18 +489,26 @@ static ssize_t local_pwritev(FsContext *ctx, V9fsFidOpenState *fs, static int local_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp) { - char buffer[PATH_MAX]; + char *buffer; + int ret = -1; char *path = fs_path->data; + buffer = rpath(fs_ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + if (fs_ctx->export_flags & V9FS_SM_MAPPED) { - return local_set_xattr(rpath(fs_ctx, path, buffer), credp); + ret = local_set_xattr(buffer, credp); } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { - return local_set_mapped_file_attr(fs_ctx, path, credp); + ret = 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); + ret = chmod(buffer, credp->fc_mode); } - return -1; + g_free(buffer); + return ret; } static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, @@ -462,28 +518,33 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, int err = -1; int serrno = 0; V9fsString fullname; - char buffer[PATH_MAX]; + char *buffer; v9fs_string_init(&fullname); v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); path = fullname.data; + buffer = rpath(fs_ctx, path); + if (!buffer) { + errno = ENOMEM; + v9fs_string_free(&fullname); + return -1; + } + /* Determine the security model */ if (fs_ctx->export_flags & V9FS_SM_MAPPED) { - err = mknod(rpath(fs_ctx, path, buffer), - SM_LOCAL_MODE_BITS|S_IFREG, 0); + err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); if (err == -1) { goto out; } - err = local_set_xattr(rpath(fs_ctx, path, buffer), credp); + err = local_set_xattr(buffer, credp); if (err == -1) { 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); + err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); if (err == -1) { goto out; } @@ -494,8 +555,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, } } 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, - credp->fc_rdev); + err = mknod(buffer, credp->fc_mode, credp->fc_rdev); if (err == -1) { goto out; } @@ -508,9 +568,10 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, goto out; err_end: - remove(rpath(fs_ctx, path, buffer)); + remove(buffer); errno = serrno; out: + g_free(buffer); v9fs_string_free(&fullname); return err; } @@ -522,26 +583,33 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, int err = -1; int serrno = 0; V9fsString fullname; - char buffer[PATH_MAX]; + char *buffer; v9fs_string_init(&fullname); v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); path = fullname.data; + buffer = rpath(fs_ctx, path); + if (!buffer) { + errno = ENOMEM; + v9fs_string_free(&fullname); + return -1; + } + /* Determine the security model */ if (fs_ctx->export_flags & V9FS_SM_MAPPED) { - err = mkdir(rpath(fs_ctx, path, buffer), SM_LOCAL_DIR_MODE_BITS); + err = mkdir(buffer, SM_LOCAL_DIR_MODE_BITS); if (err == -1) { goto out; } credp->fc_mode = credp->fc_mode|S_IFDIR; - err = local_set_xattr(rpath(fs_ctx, path, buffer), credp); + err = local_set_xattr(buffer, credp); if (err == -1) { 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); + err = mkdir(buffer, SM_LOCAL_DIR_MODE_BITS); if (err == -1) { goto out; } @@ -553,7 +621,7 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, } } 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); + err = mkdir(buffer, credp->fc_mode); if (err == -1) { goto out; } @@ -566,9 +634,10 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, goto out; err_end: - remove(rpath(fs_ctx, path, buffer)); + remove(buffer); errno = serrno; out: + g_free(buffer); v9fs_string_free(&fullname); return err; } @@ -626,7 +695,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, int err = -1; int serrno = 0; V9fsString fullname; - char buffer[PATH_MAX]; + char *buffer; /* * Mark all the open to not follow symlinks @@ -637,22 +706,29 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); path = fullname.data; + buffer = rpath(fs_ctx, path); + if (!buffer) { + errno = ENOMEM; + v9fs_string_free(&fullname); + return -1; + } + /* Determine the security model */ if (fs_ctx->export_flags & V9FS_SM_MAPPED) { - fd = open(rpath(fs_ctx, path, buffer), flags, SM_LOCAL_MODE_BITS); + fd = open(buffer, flags, SM_LOCAL_MODE_BITS); if (fd == -1) { err = fd; goto out; } credp->fc_mode = credp->fc_mode|S_IFREG; /* Set cleint credentials in xattr */ - err = local_set_xattr(rpath(fs_ctx, path, buffer), credp); + err = local_set_xattr(buffer, credp); if (err == -1) { 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); + fd = open(buffer, flags, SM_LOCAL_MODE_BITS); if (fd == -1) { err = fd; goto out; @@ -666,7 +742,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, } } 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); + fd = open(buffer, flags, credp->fc_mode); if (fd == -1) { err = fd; goto out; @@ -683,9 +759,10 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, err_end: close(fd); - remove(rpath(fs_ctx, path, buffer)); + remove(buffer); errno = serrno; out: + g_free(buffer); v9fs_string_free(&fullname); return err; } @@ -698,19 +775,24 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, int serrno = 0; char *newpath; V9fsString fullname; - char buffer[PATH_MAX]; + char *buffer; v9fs_string_init(&fullname); v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); newpath = fullname.data; + buffer = rpath(fs_ctx, newpath); + if (!buffer) { + errno = ENOMEM; + v9fs_string_free(&fullname); + return -1; + } + /* Determine the security model */ if (fs_ctx->export_flags & V9FS_SM_MAPPED) { int fd; ssize_t oldpath_size, write_size; - fd = open(rpath(fs_ctx, newpath, buffer), - O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, - SM_LOCAL_MODE_BITS); + fd = open(buffer, O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, SM_LOCAL_MODE_BITS); if (fd == -1) { err = fd; goto out; @@ -730,7 +812,7 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, 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, buffer), credp); + err = local_set_xattr(buffer, credp); if (err == -1) { serrno = errno; goto err_end; @@ -738,9 +820,7 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, } 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|O_NOFOLLOW, - SM_LOCAL_MODE_BITS); + fd = open(buffer, O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, SM_LOCAL_MODE_BITS); if (fd == -1) { err = fd; goto out; @@ -767,12 +847,11 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, } } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || (fs_ctx->export_flags & V9FS_SM_NONE)) { - err = symlink(oldpath, rpath(fs_ctx, newpath, buffer)); + err = symlink(oldpath, buffer); if (err) { goto out; } - err = lchown(rpath(fs_ctx, newpath, buffer), credp->fc_uid, - credp->fc_gid); + err = lchown(buffer, credp->fc_uid, credp->fc_gid); if (err == -1) { /* * If we fail to change ownership and if we are @@ -788,9 +867,10 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, goto out; err_end: - remove(rpath(fs_ctx, newpath, buffer)); + remove(buffer); errno = serrno; out: + g_free(buffer); v9fs_string_free(&fullname); return err; } @@ -800,13 +880,26 @@ static int local_link(FsContext *ctx, V9fsPath *oldpath, { int ret; V9fsString newpath; - char buffer[PATH_MAX], buffer1[PATH_MAX]; + char *buffer, *buffer1, *buffer0, *buffer01; v9fs_string_init(&newpath); v9fs_string_sprintf(&newpath, "%s/%s", dirpath->data, name); - ret = link(rpath(ctx, oldpath->data, buffer), - rpath(ctx, newpath.data, buffer1)); + buffer = rpath(ctx, oldpath->data); + if (!buffer) { + errno = ENOMEM; + v9fs_string_free(&newpath); + return -1; + } + buffer1 = rpath(ctx, newpath.data); + if (!buffer1) { + errno = ENOMEM; + g_free(buffer); + v9fs_string_free(&newpath); + return -1; + } + + ret = link(buffer, buffer1); /* now link the virtfs_metadata files */ if (!ret && (ctx->export_flags & V9FS_SM_MAPPED_FILE)) { @@ -815,81 +908,156 @@ static int local_link(FsContext *ctx, V9fsPath *oldpath, if (ret < 0) { goto err_out; } - ret = link(local_mapped_attr_path(ctx, oldpath->data, buffer), - local_mapped_attr_path(ctx, newpath.data, buffer1)); + buffer0 = local_mapped_attr_path(ctx, oldpath->data); + if (!buffer0) { + errno = ENOMEM; + ret = -1; + goto err_out; + } + buffer01 = local_mapped_attr_path(ctx, newpath.data); + if (!buffer01) { + errno = ENOMEM; + g_free(buffer01); + ret = -1; + goto err_out; + } + ret = link(buffer0, buffer01); + g_free(buffer0); + g_free(buffer01); if (ret < 0 && errno != ENOENT) { goto err_out; } } err_out: + g_free(buffer); + g_free(buffer1); v9fs_string_free(&newpath); return ret; } static int local_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size) { - char buffer[PATH_MAX]; + char *buffer; + int ret; char *path = fs_path->data; - return truncate(rpath(ctx, path, buffer), size); + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = truncate(buffer, size); + g_free(buffer); + return ret; } static int local_rename(FsContext *ctx, const char *oldpath, const char *newpath) { int err; - char buffer[PATH_MAX], buffer1[PATH_MAX]; + char *buffer, *buffer1, *buffer0, *buffer01; + + buffer = rpath(ctx, oldpath); + if (!buffer) { + errno = ENOMEM; + return -1; + } + buffer1 = rpath(ctx, newpath); + if (!buffer1) { + errno = ENOMEM; + g_free(buffer); + return -1; + } if (ctx->export_flags & V9FS_SM_MAPPED_FILE) { err = local_create_mapped_attr_dir(ctx, newpath); if (err < 0) { - return err; + goto ret_err; } /* rename the .virtfs_metadata files */ - err = rename(local_mapped_attr_path(ctx, oldpath, buffer), - local_mapped_attr_path(ctx, newpath, buffer1)); + buffer0 = local_mapped_attr_path(ctx, oldpath); + if (!buffer0) { + errno = ENOMEM; + err = -1; + goto ret_err; + } + buffer01 = local_mapped_attr_path(ctx, newpath); + if (!buffer01) { + errno = ENOMEM; + g_free(buffer01); + err = -1; + goto ret_err; + } + err = rename(buffer0, buffer01); + g_free(buffer0); + g_free(buffer01); if (err < 0 && errno != ENOENT) { - return err; + goto ret_err; } } - return rename(rpath(ctx, oldpath, buffer), rpath(ctx, newpath, buffer1)); + err = rename(buffer, buffer1); +ret_err: + g_free(buffer1); + g_free(buffer); + return err; } static int local_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp) { - char buffer[PATH_MAX]; + char *buffer; + int ret = -1; char *path = fs_path->data; + buffer = rpath(fs_ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } if ((credp->fc_uid == -1 && credp->fc_gid == -1) || (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || (fs_ctx->export_flags & V9FS_SM_NONE)) { - return lchown(rpath(fs_ctx, path, buffer), - credp->fc_uid, credp->fc_gid); + ret = lchown(buffer, 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); + ret = local_set_xattr(buffer, credp); } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) { - return local_set_mapped_file_attr(fs_ctx, path, credp); + ret = local_set_mapped_file_attr(fs_ctx, path, credp); } - return -1; + g_free(buffer); + return ret; } static int local_utimensat(FsContext *s, V9fsPath *fs_path, const struct timespec *buf) { - char buffer[PATH_MAX]; + char *buffer; + int ret; char *path = fs_path->data; - return qemu_utimens(rpath(s, path, buffer), buf); + buffer = rpath(s, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = qemu_utimens(buffer, buf); + g_free(buffer); + return ret; } static int local_remove(FsContext *ctx, const char *path) { int err; struct stat stbuf; - char buffer[PATH_MAX]; + char *buffer; + char *buffer1; + + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } if (ctx->export_flags & V9FS_SM_MAPPED_FILE) { - err = lstat(rpath(ctx, path, buffer), &stbuf); + err = lstat(buffer, &stbuf); if (err) { goto err_out; } @@ -898,8 +1066,15 @@ static int local_remove(FsContext *ctx, const char *path) * directory */ if (S_ISDIR(stbuf.st_mode)) { - sprintf(buffer, "%s/%s/%s", ctx->fs_root, path, VIRTFS_META_DIR); - err = remove(buffer); + buffer1 = g_strdup_printf("%s/%s/%s", ctx->fs_root, + path, VIRTFS_META_DIR); + if (!buffer1) { + errno = ENOMEM; + err = -1; + goto err_out; + } + err = remove(buffer1); + g_free(buffer1); if (err < 0 && errno != ENOENT) { /* * We didn't had the .virtfs_metadata file. May be file created @@ -912,7 +1087,14 @@ static int local_remove(FsContext *ctx, const char *path) * Now remove the name from parent directory * .virtfs_metadata directory */ - err = remove(local_mapped_attr_path(ctx, path, buffer)); + buffer1 = local_mapped_attr_path(ctx, path); + if (!buffer1) { + errno = ENOMEM; + err = -1; + goto err_out; + } + err = remove(buffer1); + g_free(buffer1); if (err < 0 && errno != ENOENT) { /* * We didn't had the .virtfs_metadata file. May be file created @@ -921,8 +1103,10 @@ static int local_remove(FsContext *ctx, const char *path) goto err_out; } } - return remove(rpath(ctx, path, buffer)); + + err = remove(buffer); err_out: + g_free(buffer); return err; } @@ -946,10 +1130,18 @@ static int local_fsync(FsContext *ctx, int fid_type, static int local_statfs(FsContext *s, V9fsPath *fs_path, struct statfs *stbuf) { - char buffer[PATH_MAX]; + char *buffer; + int ret; char *path = fs_path->data; - return statfs(rpath(s, path, buffer), stbuf); + buffer = rpath(s, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = statfs(buffer, stbuf); + g_free(buffer); + return ret; } static ssize_t local_lgetxattr(FsContext *ctx, V9fsPath *fs_path, @@ -1022,20 +1214,39 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir, { int ret; V9fsString fullname; - char buffer[PATH_MAX]; + char *buffer0 = NULL, *buffer1 = NULL, *buffer2 = NULL; v9fs_string_init(&fullname); - v9fs_string_sprintf(&fullname, "%s/%s", dir->data, name); + + buffer0 = rpath(ctx, fullname.data); + if (!buffer0) { + errno = ENOMEM; + ret = -1; + goto err_out; + } + if (ctx->export_flags & V9FS_SM_MAPPED_FILE) { + buffer1 = local_mapped_attr_path(ctx, fullname.data); + if (!buffer1) { + errno = ENOMEM; + ret = -1; + goto err_out; + } + if (flags == AT_REMOVEDIR) { /* * If directory remove .virtfs_metadata contained in the * directory */ - sprintf(buffer, "%s/%s/%s", ctx->fs_root, + buffer2 = g_strdup_printf("%s/%s/%s", ctx->fs_root, fullname.data, VIRTFS_META_DIR); - ret = remove(buffer); + if (!buffer2) { + errno = ENOMEM; + ret = -1; + goto err_out; + } + ret = remove(buffer2); if (ret < 0 && errno != ENOENT) { /* * We didn't had the .virtfs_metadata file. May be file created @@ -1048,7 +1259,7 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir, * Now remove the name from parent directory * .virtfs_metadata directory. */ - ret = remove(local_mapped_attr_path(ctx, fullname.data, buffer)); + ret = remove(buffer1); if (ret < 0 && errno != ENOENT) { /* * We didn't had the .virtfs_metadata file. May be file created @@ -1058,10 +1269,18 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir, } } /* Remove the name finally */ - ret = remove(rpath(ctx, fullname.data, buffer)); - v9fs_string_free(&fullname); - + ret = remove(buffer0); err_out: + if (buffer0) { + g_free(buffer0); + } + if (buffer1) { + g_free(buffer1); + } + if (buffer2) { + g_free(buffer2); + } + v9fs_string_free(&fullname); return ret; } diff --git a/hw/9pfs/virtio-9p-posix-acl.c b/hw/9pfs/virtio-9p-posix-acl.c index 339c5ec..ffae353 100644 --- a/hw/9pfs/virtio-9p-posix-acl.c +++ b/hw/9pfs/virtio-9p-posix-acl.c @@ -26,8 +26,17 @@ static ssize_t mp_pacl_getxattr(FsContext *ctx, const char *path, const char *name, void *value, size_t size) { - char buffer[PATH_MAX]; - return lgetxattr(rpath(ctx, path, buffer), MAP_ACL_ACCESS, value, size); + char *buffer; + ssize_t ret; + + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lgetxattr(buffer, MAP_ACL_ACCESS, value, size); + g_free(buffer); + return ret; } static ssize_t mp_pacl_listxattr(FsContext *ctx, const char *path, @@ -52,17 +61,31 @@ static ssize_t mp_pacl_listxattr(FsContext *ctx, const char *path, static int mp_pacl_setxattr(FsContext *ctx, const char *path, const char *name, void *value, size_t size, int flags) { - char buffer[PATH_MAX]; - return lsetxattr(rpath(ctx, path, buffer), MAP_ACL_ACCESS, value, - size, flags); + char *buffer; + int ret; + + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lsetxattr(buffer, MAP_ACL_ACCESS, value, size, flags); + g_free(buffer); + return ret; } static int mp_pacl_removexattr(FsContext *ctx, const char *path, const char *name) { int ret; - char buffer[PATH_MAX]; - ret = lremovexattr(rpath(ctx, path, buffer), MAP_ACL_ACCESS); + char *buffer; + + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lremovexattr(buffer, MAP_ACL_ACCESS); if (ret == -1 && errno == ENODATA) { /* * We don't get ENODATA error when trying to remove a @@ -72,14 +95,24 @@ static int mp_pacl_removexattr(FsContext *ctx, errno = 0; ret = 0; } + g_free(buffer); return ret; } static ssize_t mp_dacl_getxattr(FsContext *ctx, const char *path, const char *name, void *value, size_t size) { - char buffer[PATH_MAX]; - return lgetxattr(rpath(ctx, path, buffer), MAP_ACL_DEFAULT, value, size); + char *buffer; + ssize_t ret; + + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lgetxattr(buffer, MAP_ACL_DEFAULT, value, size); + g_free(buffer); + return ret; } static ssize_t mp_dacl_listxattr(FsContext *ctx, const char *path, @@ -104,17 +137,31 @@ static ssize_t mp_dacl_listxattr(FsContext *ctx, const char *path, static int mp_dacl_setxattr(FsContext *ctx, const char *path, const char *name, void *value, size_t size, int flags) { - char buffer[PATH_MAX]; - return lsetxattr(rpath(ctx, path, buffer), MAP_ACL_DEFAULT, value, - size, flags); + char *buffer; + int ret; + + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lsetxattr(buffer, MAP_ACL_DEFAULT, value, size, flags); + g_free(buffer); + return ret; } static int mp_dacl_removexattr(FsContext *ctx, const char *path, const char *name) { int ret; - char buffer[PATH_MAX]; - ret = lremovexattr(rpath(ctx, path, buffer), MAP_ACL_DEFAULT); + char *buffer; + + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lremovexattr(buffer, MAP_ACL_DEFAULT); if (ret == -1 && errno == ENODATA) { /* * We don't get ENODATA error when trying to remove a @@ -124,6 +171,7 @@ static int mp_dacl_removexattr(FsContext *ctx, errno = 0; ret = 0; } + g_free(buffer); return ret; } diff --git a/hw/9pfs/virtio-9p-xattr-user.c b/hw/9pfs/virtio-9p-xattr-user.c index e0c92eb..6f82710 100644 --- a/hw/9pfs/virtio-9p-xattr-user.c +++ b/hw/9pfs/virtio-9p-xattr-user.c @@ -21,7 +21,8 @@ static ssize_t mp_user_getxattr(FsContext *ctx, const char *path, const char *name, void *value, size_t size) { - char buffer[PATH_MAX]; + char *buffer; + ssize_t ret; if (strncmp(name, "user.virtfs.", 12) == 0) { /* * Don't allow fetch of user.virtfs namesapce @@ -30,7 +31,14 @@ static ssize_t mp_user_getxattr(FsContext *ctx, const char *path, errno = ENOATTR; return -1; } - return lgetxattr(rpath(ctx, path, buffer), name, value, size); + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lgetxattr(buffer, name, value, size); + g_free(buffer); + return ret; } static ssize_t mp_user_listxattr(FsContext *ctx, const char *path, @@ -69,7 +77,8 @@ static ssize_t mp_user_listxattr(FsContext *ctx, const char *path, static int mp_user_setxattr(FsContext *ctx, const char *path, const char *name, void *value, size_t size, int flags) { - char buffer[PATH_MAX]; + char *buffer; + int ret; if (strncmp(name, "user.virtfs.", 12) == 0) { /* * Don't allow fetch of user.virtfs namesapce @@ -78,13 +87,21 @@ static int mp_user_setxattr(FsContext *ctx, const char *path, const char *name, errno = EACCES; return -1; } - return lsetxattr(rpath(ctx, path, buffer), name, value, size, flags); + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lsetxattr(buffer, name, value, size, flags); + g_free(buffer); + return ret; } static int mp_user_removexattr(FsContext *ctx, const char *path, const char *name) { - char buffer[PATH_MAX]; + char *buffer; + int ret; if (strncmp(name, "user.virtfs.", 12) == 0) { /* * Don't allow fetch of user.virtfs namesapce @@ -93,7 +110,14 @@ static int mp_user_removexattr(FsContext *ctx, errno = EACCES; return -1; } - return lremovexattr(rpath(ctx, path, buffer), name); + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lremovexattr(buffer, name); + g_free(buffer); + return ret; } XattrOperations mapped_user_xattr = { diff --git a/hw/9pfs/virtio-9p-xattr.c b/hw/9pfs/virtio-9p-xattr.c index 3fae557..1f5008a 100644 --- a/hw/9pfs/virtio-9p-xattr.c +++ b/hw/9pfs/virtio-9p-xattr.c @@ -67,21 +67,28 @@ ssize_t v9fs_list_xattr(FsContext *ctx, const char *path, void *value, size_t vsize) { ssize_t size = 0; - char buffer[PATH_MAX]; + char *buffer; void *ovalue = value; XattrOperations *xops; char *orig_value, *orig_value_start; ssize_t xattr_len, parsed_len = 0, attr_len; /* Get the actual len */ - xattr_len = llistxattr(rpath(ctx, path, buffer), value, 0); + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + xattr_len = llistxattr(buffer, value, 0); if (xattr_len <= 0) { + g_free(buffer); return xattr_len; } /* Now fetch the xattr and find the actual size */ orig_value = g_malloc(xattr_len); - xattr_len = llistxattr(rpath(ctx, path, buffer), orig_value, xattr_len); + xattr_len = llistxattr(buffer, orig_value, xattr_len); + g_free(buffer); /* store the orig pointer */ orig_value_start = orig_value; diff --git a/hw/9pfs/virtio-9p-xattr.h b/hw/9pfs/virtio-9p-xattr.h index 41cc6cb..683d37b 100644 --- a/hw/9pfs/virtio-9p-xattr.h +++ b/hw/9pfs/virtio-9p-xattr.h @@ -54,23 +54,50 @@ ssize_t pt_listxattr(FsContext *ctx, const char *path, char *name, void *value, static inline ssize_t pt_getxattr(FsContext *ctx, const char *path, const char *name, void *value, size_t size) { - char buffer[PATH_MAX]; - return lgetxattr(rpath(ctx, path, buffer), name, value, size); + char *buffer; + ssize_t ret; + + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lgetxattr(buffer, name, value, size); + g_free(buffer); + return ret; } static inline int pt_setxattr(FsContext *ctx, const char *path, const char *name, void *value, size_t size, int flags) { - char buffer[PATH_MAX]; - return lsetxattr(rpath(ctx, path, buffer), name, value, size, flags); + char *buffer; + int ret; + + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lsetxattr(buffer, name, value, size, flags); + g_free(buffer); + return ret; } static inline int pt_removexattr(FsContext *ctx, const char *path, const char *name) { - char buffer[PATH_MAX]; - return lremovexattr(rpath(ctx, path, buffer), name); + char *buffer; + int ret; + + buffer = rpath(ctx, path); + if (!buffer) { + errno = ENOMEM; + return -1; + } + ret = lremovexattr(buffer, name); + g_free(buffer); + return ret; } static inline ssize_t notsup_getxattr(FsContext *ctx, const char *path, diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h index 1d6eedb..2c3603a 100644 --- a/hw/9pfs/virtio-9p.h +++ b/hw/9pfs/virtio-9p.h @@ -6,6 +6,7 @@ #include <sys/time.h> #include <utime.h> #include <sys/resource.h> +#include <glib.h> #include "hw/virtio/virtio.h" #include "fsdev/file-op-9p.h" #include "fsdev/virtio-9p-marshal.h" @@ -112,10 +113,9 @@ enum p9_proto_version { #define FID_REFERENCED 0x1 #define FID_NON_RECLAIMABLE 0x2 -static inline const char *rpath(FsContext *ctx, const char *path, char *buffer) +static inline char *rpath(FsContext *ctx, const char *path) { - snprintf(buffer, PATH_MAX, "%s/%s", ctx->fs_root, path); - return buffer; + return g_strdup_printf("%s/%s", ctx->fs_root, path); } /*
When path is truncated by PATH_MAX limitation, it causes QEMU to access incorrect file. So use original full path instead of PATH_MAX within 9pfs (need check/process ENOMEM for related memory allocation). Also find/fix several another related issues when failure occurs. - check 'fh' in handle_name_to_path(). - need call v9fs_string_free() at 'err_out:' in local_unlinkat(). - sprintf() will cause memory overflow in "virtio-9p-local.c" The related test: - Environments (for qemu-devel): - Host is under fedora17 desktop with ext4fs: qemu-system-x86_64 -hda test.img -m 1024 \ -net nic,vlan=4,model=virtio,macaddr=00:16:35:AF:94:04 \ -net tap,vlan=4,ifname=tap4,script=no,downscript=no \ -device virtio-9p-pci,id=fs0,fsdev=fsdev0,mount_tag=hostshare \ -fsdev local,security_model=passthrough,id=fsdev0,\ path=/upstream/vm/data/share/1234567890abcdefghijklmnopqrstuvwxyz\ ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890acdefghijklmnopqrstuvwxyz\ ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890/111111111111111111111111111\ 1111111111111111111111111111111111111111111111111111222222222222\ 2222222222222222222222222222222222222222222222222222222222222222\ 2222222222222222222222222222222222233333333333333333333333333333\ 3333333333333333333333333333333333 - Guest is ubuntu12 server with 9pfs. mount -t 9p -o trans=virtio,version=9p2000.L hostshare /share - Limitations: full path limitation is PATH_MAX (4096B include nul) under Linux. file/dir node name maximized length is 256 (include nul) under ext4. - Special test: Under host, modify the file: "/upstream/vm/data/share/1234567890abcdefg\ hijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890acdefghijklmno\ pqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890/111111111111111111111\ 111111111111111111111111111111111111111111111111111111111122222222222\ 222222222222222222222222222222222222222222222222222222222222222222222\ 222222222222222222222222222222233333333333333333333333333333333333333\ 3333333333333333333333333/4444444444444444444444444444444444444444444\ 444444444444444444444444444444444444444444444444444444444444444444444\ 444444444444444444444444444444444444444444444444444444444444444444444\ 444444444444444444444444444444444444444/55555555555555555555555555555\ 555555555555555555555555555555555555555555555555555555555555555555555\ 555555555555555555555555555555555555555555555555555555555555555555555\ 555555555555555555555555555555555555555555555555555555555555555555555\ 55555555/666666666666666666666666666666666666666666666666666666666666\ 666666666666666666666666666666666666666666666666666666666666666666666\ 666666666666666666666666666666666666666666666666666666666666666666666\ 666666666666666666666/77777777777777777777777777777777777777777777777\ 777777777777777777777777777777777777777777777777777777777777777777777\ 777777777777777777777777777777777777777777777777777777777777777777777\ 77777777777777777777777777777777777777777777777777777777777/888888888\ 888888888888888888888888888888888888888888888888888888888888888888888\ 888888888888888888888888888888888888888888888888888888888888888888888\ 888888888888888888888888888888888888888888888888888888888888888888888\ 888888888/99999999999999999999999999999999999999999999999999999999999\ 999999999999999999999999999999999999999999999999999999999999999999999\ 999999999999999999999999999999999999999999999999999999999999999999999\ 99999999999999999999999999999999999999999/000000000000000000000000000\ 000000000000000000000000000000000000000000000000000000000000000000000\ 000000000000000000000000000000000000000000000000000000000000000000000\ 000000000000000000000000000000000000000000000000/aaaaaaaaaaaaaaaaaaaa\ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbb\ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/ccccccccc\ ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc\ ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc\ ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc\ cccccccccc/dddddddddddddddddddddddddddddddddddddddddddddddddddddddddd\ ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd\ ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd\ dddddddddddddddddddddd/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\ eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\ eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\ eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/fffffffffffffff\ fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff/gggggggggg\ ggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg\ ggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg\ ggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg\ ggggggggggggggggggggggg/iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii\ iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii\ iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii\ iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii/jjjjjjjjjjjjj\ jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj\ jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj/ppppppppppppppppppppp\ ppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppp\ ppppppppppppppppppppppppppppppppppppppp/test1234567890file.log" (need enter dir firstly, then modify file, or can not open it). Under guest, still allow modify "test1234567890file.log" (will generate "test123456" file with contents). After apply this patch, can not open "test1234567890file.log" under guest (permission denied). - Common test: All are still OK after apply this path. "mkdir -p", "create/open file/dir", "modify file/dir", "rm file/dir". change various mount point paths under host and/or guest. Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> --- hw/9pfs/cofs.c | 22 ++- hw/9pfs/virtio-9p-handle.c | 25 ++- hw/9pfs/virtio-9p-local.c | 433 +++++++++++++++++++++++++++++++---------- hw/9pfs/virtio-9p-posix-acl.c | 76 ++++++-- hw/9pfs/virtio-9p-xattr-user.c | 36 +++- hw/9pfs/virtio-9p-xattr.c | 13 +- hw/9pfs/virtio-9p-xattr.h | 39 +++- hw/9pfs/virtio-9p.h | 6 +- 8 files changed, 504 insertions(+), 146 deletions(-)