Message ID | 20191212163904.159893-47-dgilbert@redhat.com |
---|---|
State | New |
Headers | show |
Series | virtiofs daemon [all] | expand |
On 12/12/19 5:38 PM, Dr. David Alan Gilbert (git) wrote: > From: Stefan Hajnoczi <stefanha@redhat.com> > > Sandboxing will remove /proc from the mount namespace so we can no > longer build string paths into "/proc/self/fd/...". > > Keep an O_PATH file descriptor so we can still re-open fds via > /proc/self/fd. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > tools/virtiofsd/passthrough_ll.c | 129 ++++++++++++++++++++++++------- > 1 file changed, 102 insertions(+), 27 deletions(-) > > diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c > index 123f095990..006908f25a 100644 > --- a/tools/virtiofsd/passthrough_ll.c > +++ b/tools/virtiofsd/passthrough_ll.c > @@ -110,6 +110,9 @@ struct lo_data { > struct lo_map ino_map; /* protected by lo->mutex */ > struct lo_map dirp_map; /* protected by lo->mutex */ > struct lo_map fd_map; /* protected by lo->mutex */ > + > + /* An O_PATH file descriptor to /proc/self/fd/ */ > + int proc_self_fd; > }; > > static const struct fuse_opt lo_opts[] = { > @@ -379,9 +382,9 @@ static int lo_parent_and_name(struct lo_data *lo, struct lo_inode *inode, > int res; > > retry: > - sprintf(procname, "/proc/self/fd/%i", inode->fd); > + sprintf(procname, "%i", inode->fd); > > - res = readlink(procname, path, PATH_MAX); > + res = readlinkat(lo->proc_self_fd, procname, path, PATH_MAX); > if (res < 0) { > fuse_log(FUSE_LOG_WARNING, "lo_parent_and_name: readlink failed: %m\n"); > goto fail_noretry; > @@ -477,9 +480,9 @@ static int utimensat_empty(struct lo_data *lo, struct lo_inode *inode, > } > return res; > } > - sprintf(path, "/proc/self/fd/%i", inode->fd); > + sprintf(path, "%i", inode->fd); > > - return utimensat(AT_FDCWD, path, tv, 0); > + return utimensat(lo->proc_self_fd, path, tv, 0); > > fallback: > res = lo_parent_and_name(lo, inode, path, &parent); > @@ -535,8 +538,8 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, > if (fi) { > res = fchmod(fd, attr->st_mode); > } else { > - sprintf(procname, "/proc/self/fd/%i", ifd); > - res = chmod(procname, attr->st_mode); > + sprintf(procname, "%i", ifd); > + res = fchmodat(lo->proc_self_fd, procname, attr->st_mode, 0); > } > if (res == -1) { > goto out_err; > @@ -552,11 +555,23 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, > } > } > if (valid & FUSE_SET_ATTR_SIZE) { > + int truncfd; > + > if (fi) { > - res = ftruncate(fd, attr->st_size); > + truncfd = fd; > } else { > - sprintf(procname, "/proc/self/fd/%i", ifd); > - res = truncate(procname, attr->st_size); > + sprintf(procname, "%i", ifd); > + truncfd = openat(lo->proc_self_fd, procname, O_RDWR); > + if (truncfd < 0) { > + goto out_err; > + } > + } > + > + res = ftruncate(truncfd, attr->st_size); > + if (!fi) { > + saverr = errno; > + close(truncfd); > + errno = saverr; > } > if (res == -1) { > goto out_err; > @@ -857,9 +872,9 @@ static int linkat_empty_nofollow(struct lo_data *lo, struct lo_inode *inode, > return res; > } > > - sprintf(path, "/proc/self/fd/%i", inode->fd); > + sprintf(path, "%i", inode->fd); > > - return linkat(AT_FDCWD, path, dfd, name, AT_SYMLINK_FOLLOW); > + return linkat(lo->proc_self_fd, path, dfd, name, AT_SYMLINK_FOLLOW); > > fallback: > res = lo_parent_and_name(lo, inode, path, &parent); > @@ -1387,8 +1402,8 @@ static void lo_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi) > fi->flags &= ~O_APPEND; > } > > - sprintf(buf, "/proc/self/fd/%i", lo_fd(req, ino)); > - fd = open(buf, fi->flags & ~O_NOFOLLOW); > + sprintf(buf, "%i", lo_fd(req, ino)); > + fd = openat(lo->proc_self_fd, buf, fi->flags & ~O_NOFOLLOW); > if (fd == -1) { > return (void)fuse_reply_err(req, errno); > } > @@ -1440,8 +1455,8 @@ static void lo_flush(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi) > static void lo_fsync(fuse_req_t req, fuse_ino_t ino, int datasync, > struct fuse_file_info *fi) > { > + struct lo_data *lo = lo_data(req); We can initialize this one ... > int res; > - (void)ino; > int fd; > char *buf; > > @@ -1449,12 +1464,12 @@ static void lo_fsync(fuse_req_t req, fuse_ino_t ino, int datasync, > (void *)fi); > > if (!fi) { ... here: lo = lo_data(req); Similarly in other functions, but I see this is the style used by this file. > - res = asprintf(&buf, "/proc/self/fd/%i", lo_fd(req, ino)); > + res = asprintf(&buf, "%i", lo_fd(req, ino)); > if (res == -1) { > return (void)fuse_reply_err(req, errno); > } > > - fd = open(buf, O_RDWR); > + fd = openat(lo->proc_self_fd, buf, O_RDWR); > free(buf); > if (fd == -1) { > return (void)fuse_reply_err(req, errno); > @@ -1570,11 +1585,13 @@ static void lo_flock(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi, > static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > size_t size) > { > + struct lo_data *lo = lo_data(req); > char *value = NULL; > char procname[64]; > struct lo_inode *inode; > ssize_t ret; > int saverr; > + int fd = -1; > > inode = lo_inode(req, ino); > if (!inode) { > @@ -1599,7 +1616,11 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > goto out; > } > > - sprintf(procname, "/proc/self/fd/%i", inode->fd); > + sprintf(procname, "%i", inode->fd); > + fd = openat(lo->proc_self_fd, procname, O_RDONLY); > + if (fd < 0) { > + goto out_err; > + } > > if (size) { > value = malloc(size); > @@ -1607,7 +1628,7 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > goto out_err; > } > > - ret = getxattr(procname, name, value, size); > + ret = fgetxattr(fd, name, value, size); > if (ret == -1) { > goto out_err; > } > @@ -1618,7 +1639,7 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > > fuse_reply_buf(req, value, ret); > } else { > - ret = getxattr(procname, name, NULL, 0); > + ret = fgetxattr(fd, name, NULL, 0); > if (ret == -1) { > goto out_err; > } > @@ -1627,6 +1648,10 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > } > out_free: > free(value); > + > + if (fd >= 0) { > + close(fd); > + } > return; > > out_err: > @@ -1638,11 +1663,13 @@ out: > > static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) > { > + struct lo_data *lo = lo_data(req); > char *value = NULL; > char procname[64]; > struct lo_inode *inode; > ssize_t ret; > int saverr; > + int fd = -1; > > inode = lo_inode(req, ino); > if (!inode) { > @@ -1666,7 +1693,11 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) > goto out; > } > > - sprintf(procname, "/proc/self/fd/%i", inode->fd); > + sprintf(procname, "%i", inode->fd); > + fd = openat(lo->proc_self_fd, procname, O_RDONLY); > + if (fd < 0) { > + goto out_err; > + } > > if (size) { > value = malloc(size); > @@ -1674,7 +1705,7 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) > goto out_err; > } > > - ret = listxattr(procname, value, size); > + ret = flistxattr(fd, value, size); > if (ret == -1) { > goto out_err; > } > @@ -1685,7 +1716,7 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) > > fuse_reply_buf(req, value, ret); > } else { > - ret = listxattr(procname, NULL, 0); > + ret = flistxattr(fd, NULL, 0); > if (ret == -1) { > goto out_err; > } > @@ -1694,6 +1725,10 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) > } > out_free: > free(value); > + > + if (fd >= 0) { > + close(fd); > + } > return; > > out_err: > @@ -1707,9 +1742,11 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > const char *value, size_t size, int flags) > { > char procname[64]; > + struct lo_data *lo = lo_data(req); > struct lo_inode *inode; > ssize_t ret; > int saverr; > + int fd = -1; > > inode = lo_inode(req, ino); > if (!inode) { > @@ -1734,21 +1771,31 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > goto out; > } > > - sprintf(procname, "/proc/self/fd/%i", inode->fd); > + sprintf(procname, "%i", inode->fd); > + fd = openat(lo->proc_self_fd, procname, O_RDWR); > + if (fd < 0) { > + saverr = errno; > + goto out; > + } > > - ret = setxattr(procname, name, value, size, flags); > + ret = fsetxattr(fd, name, value, size, flags); > saverr = ret == -1 ? errno : 0; > > out: > + if (fd >= 0) { > + close(fd); > + } > fuse_reply_err(req, saverr); > } > > static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *name) > { > char procname[64]; > + struct lo_data *lo = lo_data(req); > struct lo_inode *inode; > ssize_t ret; > int saverr; > + int fd = -1; > > inode = lo_inode(req, ino); > if (!inode) { > @@ -1772,12 +1819,20 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *name) > goto out; > } > > - sprintf(procname, "/proc/self/fd/%i", inode->fd); > + sprintf(procname, "%i", inode->fd); > + fd = openat(lo->proc_self_fd, procname, O_RDWR); > + if (fd < 0) { > + saverr = errno; > + goto out; > + } > > - ret = removexattr(procname, name); > + ret = fremovexattr(fd, name); > saverr = ret == -1 ? errno : 0; > > out: > + if (fd >= 0) { > + close(fd); > + } > fuse_reply_err(req, saverr); > } > > @@ -1870,12 +1925,25 @@ static void print_capabilities(void) > printf("}\n"); > } > > +static void setup_proc_self_fd(struct lo_data *lo) > +{ > + lo->proc_self_fd = open("/proc/self/fd", O_PATH); > + if (lo->proc_self_fd == -1) { > + fuse_log(FUSE_LOG_ERR, "open(/proc/self/fd, O_PATH): %m\n"); > + exit(1); > + } > +} > + > int main(int argc, char *argv[]) > { > struct fuse_args args = FUSE_ARGS_INIT(argc, argv); > struct fuse_session *se; > struct fuse_cmdline_opts opts; > - struct lo_data lo = { .debug = 0, .writeback = 0 }; > + struct lo_data lo = { > + .debug = 0, > + .writeback = 0, > + .proc_self_fd = -1, > + }; > struct lo_map_elem *root_elem; > int ret = -1; > > @@ -1986,6 +2054,9 @@ int main(int argc, char *argv[]) > > fuse_daemonize(opts.foreground); > > + /* Must be after daemonize to get the right /proc/self/fd */ > + setup_proc_self_fd(&lo); > + > /* Block until ctrl+c or fusermount -u */ > ret = virtio_loop(se); > > @@ -2001,6 +2072,10 @@ err_out1: > lo_map_destroy(&lo.dirp_map); > lo_map_destroy(&lo.ino_map); > > + if (lo.proc_self_fd >= 0) { > + close(lo.proc_self_fd); > + } > + > if (lo.root.fd >= 0) { > close(lo.root.fd); > } > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote: > On 12/12/19 5:38 PM, Dr. David Alan Gilbert (git) wrote: > > From: Stefan Hajnoczi <stefanha@redhat.com> > > > > Sandboxing will remove /proc from the mount namespace so we can no > > longer build string paths into "/proc/self/fd/...". > > > > Keep an O_PATH file descriptor so we can still re-open fds via > > /proc/self/fd. > > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > --- > > tools/virtiofsd/passthrough_ll.c | 129 ++++++++++++++++++++++++------- > > 1 file changed, 102 insertions(+), 27 deletions(-) > > > > diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c > > index 123f095990..006908f25a 100644 > > --- a/tools/virtiofsd/passthrough_ll.c > > +++ b/tools/virtiofsd/passthrough_ll.c > > @@ -110,6 +110,9 @@ struct lo_data { > > struct lo_map ino_map; /* protected by lo->mutex */ > > struct lo_map dirp_map; /* protected by lo->mutex */ > > struct lo_map fd_map; /* protected by lo->mutex */ > > + > > + /* An O_PATH file descriptor to /proc/self/fd/ */ > > + int proc_self_fd; > > }; > > static const struct fuse_opt lo_opts[] = { > > @@ -379,9 +382,9 @@ static int lo_parent_and_name(struct lo_data *lo, struct lo_inode *inode, > > int res; > > retry: > > - sprintf(procname, "/proc/self/fd/%i", inode->fd); > > + sprintf(procname, "%i", inode->fd); > > - res = readlink(procname, path, PATH_MAX); > > + res = readlinkat(lo->proc_self_fd, procname, path, PATH_MAX); > > if (res < 0) { > > fuse_log(FUSE_LOG_WARNING, "lo_parent_and_name: readlink failed: %m\n"); > > goto fail_noretry; > > @@ -477,9 +480,9 @@ static int utimensat_empty(struct lo_data *lo, struct lo_inode *inode, > > } > > return res; > > } > > - sprintf(path, "/proc/self/fd/%i", inode->fd); > > + sprintf(path, "%i", inode->fd); > > - return utimensat(AT_FDCWD, path, tv, 0); > > + return utimensat(lo->proc_self_fd, path, tv, 0); > > fallback: > > res = lo_parent_and_name(lo, inode, path, &parent); > > @@ -535,8 +538,8 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, > > if (fi) { > > res = fchmod(fd, attr->st_mode); > > } else { > > - sprintf(procname, "/proc/self/fd/%i", ifd); > > - res = chmod(procname, attr->st_mode); > > + sprintf(procname, "%i", ifd); > > + res = fchmodat(lo->proc_self_fd, procname, attr->st_mode, 0); > > } > > if (res == -1) { > > goto out_err; > > @@ -552,11 +555,23 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, > > } > > } > > if (valid & FUSE_SET_ATTR_SIZE) { > > + int truncfd; > > + > > if (fi) { > > - res = ftruncate(fd, attr->st_size); > > + truncfd = fd; > > } else { > > - sprintf(procname, "/proc/self/fd/%i", ifd); > > - res = truncate(procname, attr->st_size); > > + sprintf(procname, "%i", ifd); > > + truncfd = openat(lo->proc_self_fd, procname, O_RDWR); > > + if (truncfd < 0) { > > + goto out_err; > > + } > > + } > > + > > + res = ftruncate(truncfd, attr->st_size); > > + if (!fi) { > > + saverr = errno; > > + close(truncfd); > > + errno = saverr; > > } > > if (res == -1) { > > goto out_err; > > @@ -857,9 +872,9 @@ static int linkat_empty_nofollow(struct lo_data *lo, struct lo_inode *inode, > > return res; > > } > > - sprintf(path, "/proc/self/fd/%i", inode->fd); > > + sprintf(path, "%i", inode->fd); > > - return linkat(AT_FDCWD, path, dfd, name, AT_SYMLINK_FOLLOW); > > + return linkat(lo->proc_self_fd, path, dfd, name, AT_SYMLINK_FOLLOW); > > fallback: > > res = lo_parent_and_name(lo, inode, path, &parent); > > @@ -1387,8 +1402,8 @@ static void lo_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi) > > fi->flags &= ~O_APPEND; > > } > > - sprintf(buf, "/proc/self/fd/%i", lo_fd(req, ino)); > > - fd = open(buf, fi->flags & ~O_NOFOLLOW); > > + sprintf(buf, "%i", lo_fd(req, ino)); > > + fd = openat(lo->proc_self_fd, buf, fi->flags & ~O_NOFOLLOW); > > if (fd == -1) { > > return (void)fuse_reply_err(req, errno); > > } > > @@ -1440,8 +1455,8 @@ static void lo_flush(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi) > > static void lo_fsync(fuse_req_t req, fuse_ino_t ino, int datasync, > > struct fuse_file_info *fi) > > { > > + struct lo_data *lo = lo_data(req); > > We can initialize this one ... > > > int res; > > - (void)ino; > > int fd; > > char *buf; > > @@ -1449,12 +1464,12 @@ static void lo_fsync(fuse_req_t req, fuse_ino_t ino, int datasync, > > (void *)fi); > > if (!fi) { > > ... here: > > lo = lo_data(req); > > Similarly in other functions, but I see this is the style used by this file. I moved that one, I'll keep my out for others; it's nice to keep the scope small. > > - res = asprintf(&buf, "/proc/self/fd/%i", lo_fd(req, ino)); > > + res = asprintf(&buf, "%i", lo_fd(req, ino)); > > if (res == -1) { > > return (void)fuse_reply_err(req, errno); > > } > > - fd = open(buf, O_RDWR); > > + fd = openat(lo->proc_self_fd, buf, O_RDWR); > > free(buf); > > if (fd == -1) { > > return (void)fuse_reply_err(req, errno); > > @@ -1570,11 +1585,13 @@ static void lo_flock(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi, > > static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > > size_t size) > > { > > + struct lo_data *lo = lo_data(req); > > char *value = NULL; > > char procname[64]; > > struct lo_inode *inode; > > ssize_t ret; > > int saverr; > > + int fd = -1; > > inode = lo_inode(req, ino); > > if (!inode) { > > @@ -1599,7 +1616,11 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > > goto out; > > } > > - sprintf(procname, "/proc/self/fd/%i", inode->fd); > > + sprintf(procname, "%i", inode->fd); > > + fd = openat(lo->proc_self_fd, procname, O_RDONLY); > > + if (fd < 0) { > > + goto out_err; > > + } > > if (size) { > > value = malloc(size); > > @@ -1607,7 +1628,7 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > > goto out_err; > > } > > - ret = getxattr(procname, name, value, size); > > + ret = fgetxattr(fd, name, value, size); > > if (ret == -1) { > > goto out_err; > > } > > @@ -1618,7 +1639,7 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > > fuse_reply_buf(req, value, ret); > > } else { > > - ret = getxattr(procname, name, NULL, 0); > > + ret = fgetxattr(fd, name, NULL, 0); > > if (ret == -1) { > > goto out_err; > > } > > @@ -1627,6 +1648,10 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > > } > > out_free: > > free(value); > > + > > + if (fd >= 0) { > > + close(fd); > > + } > > return; > > out_err: > > @@ -1638,11 +1663,13 @@ out: > > static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) > > { > > + struct lo_data *lo = lo_data(req); > > char *value = NULL; > > char procname[64]; > > struct lo_inode *inode; > > ssize_t ret; > > int saverr; > > + int fd = -1; > > inode = lo_inode(req, ino); > > if (!inode) { > > @@ -1666,7 +1693,11 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) > > goto out; > > } > > - sprintf(procname, "/proc/self/fd/%i", inode->fd); > > + sprintf(procname, "%i", inode->fd); > > + fd = openat(lo->proc_self_fd, procname, O_RDONLY); > > + if (fd < 0) { > > + goto out_err; > > + } > > if (size) { > > value = malloc(size); > > @@ -1674,7 +1705,7 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) > > goto out_err; > > } > > - ret = listxattr(procname, value, size); > > + ret = flistxattr(fd, value, size); > > if (ret == -1) { > > goto out_err; > > } > > @@ -1685,7 +1716,7 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) > > fuse_reply_buf(req, value, ret); > > } else { > > - ret = listxattr(procname, NULL, 0); > > + ret = flistxattr(fd, NULL, 0); > > if (ret == -1) { > > goto out_err; > > } > > @@ -1694,6 +1725,10 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) > > } > > out_free: > > free(value); > > + > > + if (fd >= 0) { > > + close(fd); > > + } > > return; > > out_err: > > @@ -1707,9 +1742,11 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > > const char *value, size_t size, int flags) > > { > > char procname[64]; > > + struct lo_data *lo = lo_data(req); > > struct lo_inode *inode; > > ssize_t ret; > > int saverr; > > + int fd = -1; > > inode = lo_inode(req, ino); > > if (!inode) { > > @@ -1734,21 +1771,31 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name, > > goto out; > > } > > - sprintf(procname, "/proc/self/fd/%i", inode->fd); > > + sprintf(procname, "%i", inode->fd); > > + fd = openat(lo->proc_self_fd, procname, O_RDWR); > > + if (fd < 0) { > > + saverr = errno; > > + goto out; > > + } > > - ret = setxattr(procname, name, value, size, flags); > > + ret = fsetxattr(fd, name, value, size, flags); > > saverr = ret == -1 ? errno : 0; > > out: > > + if (fd >= 0) { > > + close(fd); > > + } > > fuse_reply_err(req, saverr); > > } > > static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *name) > > { > > char procname[64]; > > + struct lo_data *lo = lo_data(req); > > struct lo_inode *inode; > > ssize_t ret; > > int saverr; > > + int fd = -1; > > inode = lo_inode(req, ino); > > if (!inode) { > > @@ -1772,12 +1819,20 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *name) > > goto out; > > } > > - sprintf(procname, "/proc/self/fd/%i", inode->fd); > > + sprintf(procname, "%i", inode->fd); > > + fd = openat(lo->proc_self_fd, procname, O_RDWR); > > + if (fd < 0) { > > + saverr = errno; > > + goto out; > > + } > > - ret = removexattr(procname, name); > > + ret = fremovexattr(fd, name); > > saverr = ret == -1 ? errno : 0; > > out: > > + if (fd >= 0) { > > + close(fd); > > + } > > fuse_reply_err(req, saverr); > > } > > @@ -1870,12 +1925,25 @@ static void print_capabilities(void) > > printf("}\n"); > > } > > +static void setup_proc_self_fd(struct lo_data *lo) > > +{ > > + lo->proc_self_fd = open("/proc/self/fd", O_PATH); > > + if (lo->proc_self_fd == -1) { > > + fuse_log(FUSE_LOG_ERR, "open(/proc/self/fd, O_PATH): %m\n"); > > + exit(1); > > + } > > +} > > + > > int main(int argc, char *argv[]) > > { > > struct fuse_args args = FUSE_ARGS_INIT(argc, argv); > > struct fuse_session *se; > > struct fuse_cmdline_opts opts; > > - struct lo_data lo = { .debug = 0, .writeback = 0 }; > > + struct lo_data lo = { > > + .debug = 0, > > + .writeback = 0, > > + .proc_self_fd = -1, > > + }; > > struct lo_map_elem *root_elem; > > int ret = -1; > > @@ -1986,6 +2054,9 @@ int main(int argc, char *argv[]) > > fuse_daemonize(opts.foreground); > > + /* Must be after daemonize to get the right /proc/self/fd */ > > + setup_proc_self_fd(&lo); > > + > > /* Block until ctrl+c or fusermount -u */ > > ret = virtio_loop(se); > > @@ -2001,6 +2072,10 @@ err_out1: > > lo_map_destroy(&lo.dirp_map); > > lo_map_destroy(&lo.ino_map); > > + if (lo.proc_self_fd >= 0) { > > + close(lo.proc_self_fd); > > + } > > + > > if (lo.root.fd >= 0) { > > close(lo.root.fd); > > } > > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Thanks! -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 123f095990..006908f25a 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -110,6 +110,9 @@ struct lo_data { struct lo_map ino_map; /* protected by lo->mutex */ struct lo_map dirp_map; /* protected by lo->mutex */ struct lo_map fd_map; /* protected by lo->mutex */ + + /* An O_PATH file descriptor to /proc/self/fd/ */ + int proc_self_fd; }; static const struct fuse_opt lo_opts[] = { @@ -379,9 +382,9 @@ static int lo_parent_and_name(struct lo_data *lo, struct lo_inode *inode, int res; retry: - sprintf(procname, "/proc/self/fd/%i", inode->fd); + sprintf(procname, "%i", inode->fd); - res = readlink(procname, path, PATH_MAX); + res = readlinkat(lo->proc_self_fd, procname, path, PATH_MAX); if (res < 0) { fuse_log(FUSE_LOG_WARNING, "lo_parent_and_name: readlink failed: %m\n"); goto fail_noretry; @@ -477,9 +480,9 @@ static int utimensat_empty(struct lo_data *lo, struct lo_inode *inode, } return res; } - sprintf(path, "/proc/self/fd/%i", inode->fd); + sprintf(path, "%i", inode->fd); - return utimensat(AT_FDCWD, path, tv, 0); + return utimensat(lo->proc_self_fd, path, tv, 0); fallback: res = lo_parent_and_name(lo, inode, path, &parent); @@ -535,8 +538,8 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, if (fi) { res = fchmod(fd, attr->st_mode); } else { - sprintf(procname, "/proc/self/fd/%i", ifd); - res = chmod(procname, attr->st_mode); + sprintf(procname, "%i", ifd); + res = fchmodat(lo->proc_self_fd, procname, attr->st_mode, 0); } if (res == -1) { goto out_err; @@ -552,11 +555,23 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr, } } if (valid & FUSE_SET_ATTR_SIZE) { + int truncfd; + if (fi) { - res = ftruncate(fd, attr->st_size); + truncfd = fd; } else { - sprintf(procname, "/proc/self/fd/%i", ifd); - res = truncate(procname, attr->st_size); + sprintf(procname, "%i", ifd); + truncfd = openat(lo->proc_self_fd, procname, O_RDWR); + if (truncfd < 0) { + goto out_err; + } + } + + res = ftruncate(truncfd, attr->st_size); + if (!fi) { + saverr = errno; + close(truncfd); + errno = saverr; } if (res == -1) { goto out_err; @@ -857,9 +872,9 @@ static int linkat_empty_nofollow(struct lo_data *lo, struct lo_inode *inode, return res; } - sprintf(path, "/proc/self/fd/%i", inode->fd); + sprintf(path, "%i", inode->fd); - return linkat(AT_FDCWD, path, dfd, name, AT_SYMLINK_FOLLOW); + return linkat(lo->proc_self_fd, path, dfd, name, AT_SYMLINK_FOLLOW); fallback: res = lo_parent_and_name(lo, inode, path, &parent); @@ -1387,8 +1402,8 @@ static void lo_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi) fi->flags &= ~O_APPEND; } - sprintf(buf, "/proc/self/fd/%i", lo_fd(req, ino)); - fd = open(buf, fi->flags & ~O_NOFOLLOW); + sprintf(buf, "%i", lo_fd(req, ino)); + fd = openat(lo->proc_self_fd, buf, fi->flags & ~O_NOFOLLOW); if (fd == -1) { return (void)fuse_reply_err(req, errno); } @@ -1440,8 +1455,8 @@ static void lo_flush(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi) static void lo_fsync(fuse_req_t req, fuse_ino_t ino, int datasync, struct fuse_file_info *fi) { + struct lo_data *lo = lo_data(req); int res; - (void)ino; int fd; char *buf; @@ -1449,12 +1464,12 @@ static void lo_fsync(fuse_req_t req, fuse_ino_t ino, int datasync, (void *)fi); if (!fi) { - res = asprintf(&buf, "/proc/self/fd/%i", lo_fd(req, ino)); + res = asprintf(&buf, "%i", lo_fd(req, ino)); if (res == -1) { return (void)fuse_reply_err(req, errno); } - fd = open(buf, O_RDWR); + fd = openat(lo->proc_self_fd, buf, O_RDWR); free(buf); if (fd == -1) { return (void)fuse_reply_err(req, errno); @@ -1570,11 +1585,13 @@ static void lo_flock(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi, static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, size_t size) { + struct lo_data *lo = lo_data(req); char *value = NULL; char procname[64]; struct lo_inode *inode; ssize_t ret; int saverr; + int fd = -1; inode = lo_inode(req, ino); if (!inode) { @@ -1599,7 +1616,11 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, goto out; } - sprintf(procname, "/proc/self/fd/%i", inode->fd); + sprintf(procname, "%i", inode->fd); + fd = openat(lo->proc_self_fd, procname, O_RDONLY); + if (fd < 0) { + goto out_err; + } if (size) { value = malloc(size); @@ -1607,7 +1628,7 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, goto out_err; } - ret = getxattr(procname, name, value, size); + ret = fgetxattr(fd, name, value, size); if (ret == -1) { goto out_err; } @@ -1618,7 +1639,7 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, fuse_reply_buf(req, value, ret); } else { - ret = getxattr(procname, name, NULL, 0); + ret = fgetxattr(fd, name, NULL, 0); if (ret == -1) { goto out_err; } @@ -1627,6 +1648,10 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name, } out_free: free(value); + + if (fd >= 0) { + close(fd); + } return; out_err: @@ -1638,11 +1663,13 @@ out: static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) { + struct lo_data *lo = lo_data(req); char *value = NULL; char procname[64]; struct lo_inode *inode; ssize_t ret; int saverr; + int fd = -1; inode = lo_inode(req, ino); if (!inode) { @@ -1666,7 +1693,11 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) goto out; } - sprintf(procname, "/proc/self/fd/%i", inode->fd); + sprintf(procname, "%i", inode->fd); + fd = openat(lo->proc_self_fd, procname, O_RDONLY); + if (fd < 0) { + goto out_err; + } if (size) { value = malloc(size); @@ -1674,7 +1705,7 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) goto out_err; } - ret = listxattr(procname, value, size); + ret = flistxattr(fd, value, size); if (ret == -1) { goto out_err; } @@ -1685,7 +1716,7 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) fuse_reply_buf(req, value, ret); } else { - ret = listxattr(procname, NULL, 0); + ret = flistxattr(fd, NULL, 0); if (ret == -1) { goto out_err; } @@ -1694,6 +1725,10 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) } out_free: free(value); + + if (fd >= 0) { + close(fd); + } return; out_err: @@ -1707,9 +1742,11 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name, const char *value, size_t size, int flags) { char procname[64]; + struct lo_data *lo = lo_data(req); struct lo_inode *inode; ssize_t ret; int saverr; + int fd = -1; inode = lo_inode(req, ino); if (!inode) { @@ -1734,21 +1771,31 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name, goto out; } - sprintf(procname, "/proc/self/fd/%i", inode->fd); + sprintf(procname, "%i", inode->fd); + fd = openat(lo->proc_self_fd, procname, O_RDWR); + if (fd < 0) { + saverr = errno; + goto out; + } - ret = setxattr(procname, name, value, size, flags); + ret = fsetxattr(fd, name, value, size, flags); saverr = ret == -1 ? errno : 0; out: + if (fd >= 0) { + close(fd); + } fuse_reply_err(req, saverr); } static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *name) { char procname[64]; + struct lo_data *lo = lo_data(req); struct lo_inode *inode; ssize_t ret; int saverr; + int fd = -1; inode = lo_inode(req, ino); if (!inode) { @@ -1772,12 +1819,20 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *name) goto out; } - sprintf(procname, "/proc/self/fd/%i", inode->fd); + sprintf(procname, "%i", inode->fd); + fd = openat(lo->proc_self_fd, procname, O_RDWR); + if (fd < 0) { + saverr = errno; + goto out; + } - ret = removexattr(procname, name); + ret = fremovexattr(fd, name); saverr = ret == -1 ? errno : 0; out: + if (fd >= 0) { + close(fd); + } fuse_reply_err(req, saverr); } @@ -1870,12 +1925,25 @@ static void print_capabilities(void) printf("}\n"); } +static void setup_proc_self_fd(struct lo_data *lo) +{ + lo->proc_self_fd = open("/proc/self/fd", O_PATH); + if (lo->proc_self_fd == -1) { + fuse_log(FUSE_LOG_ERR, "open(/proc/self/fd, O_PATH): %m\n"); + exit(1); + } +} + int main(int argc, char *argv[]) { struct fuse_args args = FUSE_ARGS_INIT(argc, argv); struct fuse_session *se; struct fuse_cmdline_opts opts; - struct lo_data lo = { .debug = 0, .writeback = 0 }; + struct lo_data lo = { + .debug = 0, + .writeback = 0, + .proc_self_fd = -1, + }; struct lo_map_elem *root_elem; int ret = -1; @@ -1986,6 +2054,9 @@ int main(int argc, char *argv[]) fuse_daemonize(opts.foreground); + /* Must be after daemonize to get the right /proc/self/fd */ + setup_proc_self_fd(&lo); + /* Block until ctrl+c or fusermount -u */ ret = virtio_loop(se); @@ -2001,6 +2072,10 @@ err_out1: lo_map_destroy(&lo.dirp_map); lo_map_destroy(&lo.ino_map); + if (lo.proc_self_fd >= 0) { + close(lo.proc_self_fd); + } + if (lo.root.fd >= 0) { close(lo.root.fd); }