From 0adb3aff3932d05b069bd2cb13480f1611cce654 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 9 Apr 2021 12:06:27 +0200 Subject: virtiofsd: Fix side-effect in assert() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is bad practice to put an expression with a side-effect in assert() because the side-effect won't happen if the code is compiled with -DNDEBUG. Use an intermediate variable. Consolidate this in an macro to have proper line numbers when the assertion is hit. virtiofsd: ../../tools/virtiofsd/passthrough_ll.c:2797: lo_getxattr: Assertion `fchdir_res == 0' failed. Aborted 2796 /* fchdir should not fail here */ =>2797 FCHDIR_NOFAIL(lo->proc_self_fd); 2798 ret = getxattr(procname, name, value, size); 2799 FCHDIR_NOFAIL(lo->root.fd); Fixes: bdfd66788349 ("virtiofsd: Fix xattr operations") Cc: misono.tomohiro@jp.fujitsu.com Signed-off-by: Greg Kurz Message-Id: <20210409100627.451573-1-groug@kaod.org> Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Philippe Mathieu-Daudé --- tools/virtiofsd/passthrough_ll.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 1553d2ef45..6592f96f68 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -2723,6 +2723,11 @@ static int xattr_map_server(const struct lo_data *lo, const char *server_name, return -ENODATA; } +#define FCHDIR_NOFAIL(fd) do { \ + int fchdir_res = fchdir(fd); \ + assert(fchdir_res == 0); \ + } while (0) + static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, size_t size) { @@ -2789,9 +2794,9 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, ret = fgetxattr(fd, name, value, size); } else { /* fchdir should not fail here */ - assert(fchdir(lo->proc_self_fd) == 0); + FCHDIR_NOFAIL(lo->proc_self_fd); ret = getxattr(procname, name, value, size); - assert(fchdir(lo->root.fd) == 0); + FCHDIR_NOFAIL(lo->root.fd); } if (ret == -1) { @@ -2864,9 +2869,9 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) ret = flistxattr(fd, value, size); } else { /* fchdir should not fail here */ - assert(fchdir(lo->proc_self_fd) == 0); + FCHDIR_NOFAIL(lo->proc_self_fd); ret = listxattr(procname, value, size); - assert(fchdir(lo->root.fd) == 0); + FCHDIR_NOFAIL(lo->root.fd); } if (ret == -1) { @@ -3000,9 +3005,9 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, ret = fsetxattr(fd, name, value, size, flags); } else { /* fchdir should not fail here */ - assert(fchdir(lo->proc_self_fd) == 0); + FCHDIR_NOFAIL(lo->proc_self_fd); ret = setxattr(procname, name, value, size, flags); - assert(fchdir(lo->root.fd) == 0); + FCHDIR_NOFAIL(lo->root.fd); } saverr = ret == -1 ? errno : 0; @@ -3066,9 +3071,9 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name) ret = fremovexattr(fd, name); } else { /* fchdir should not fail here */ - assert(fchdir(lo->proc_self_fd) == 0); + FCHDIR_NOFAIL(lo->proc_self_fd); ret = removexattr(procname, name); - assert(fchdir(lo->root.fd) == 0); + FCHDIR_NOFAIL(lo->root.fd); } saverr = ret == -1 ? errno : 0; -- cgit v1.2.3 From a87d29e0d7879c7284a9441de1252f458bca6c2e Mon Sep 17 00:00:00 2001 From: Carlos Venegas Date: Wed, 14 Apr 2021 20:12:06 +0000 Subject: virtiofsd: Allow use "-o xattrmap" without "-o xattr" When -o xattrmap is used, it will not work unless xattr is enabled. This patch enables xattr when -o xattrmap is used. Signed-off-by: Carlos Venegas Message-Id: <20210414201207.3612432-2-jose.carlos.venegas.munoz@intel.com> Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Connor Kuehl --- tools/virtiofsd/passthrough_ll.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 6592f96f68..2c36f4ec46 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -3831,6 +3831,7 @@ int main(int argc, char *argv[]) } if (lo.xattrmap) { + lo.xattr = 1; parse_xattrmap(&lo); } -- cgit v1.2.3 From 1221a929be9c91e679f1983c5f1af5052f309d60 Mon Sep 17 00:00:00 2001 From: Carlos Venegas Date: Wed, 14 Apr 2021 20:12:07 +0000 Subject: virtiofsd: Add help for -o xattr-mapping The option is not documented in help. Add small help about the option. Signed-off-by: Carlos Venegas Message-Id: <20210414201207.3612432-3-jose.carlos.venegas.munoz@intel.com> Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Connor Kuehl --- tools/virtiofsd/helper.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c index 28243b51b2..5e98ed702b 100644 --- a/tools/virtiofsd/helper.c +++ b/tools/virtiofsd/helper.c @@ -172,6 +172,9 @@ void fuse_cmdline_help(void) " default: no_writeback\n" " -o xattr|no_xattr enable/disable xattr\n" " default: no_xattr\n" + " -o xattrmap= Enable xattr mapping (enables xattr)\n" + " is a string consists of a series of rules\n" + " e.g. -o xattrmap=:map::user.virtiofs.:\n" " -o modcaps=CAPLIST Modify the list of capabilities\n" " e.g. -o modcaps=+sys_admin:-chown\n" " --rlimit-nofile= set maximum number of file descriptors\n" -- cgit v1.2.3 From d02a3c5a1b3d579eec0531b9453a6beb5189edad Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Wed, 28 Apr 2021 12:00:35 +0100 Subject: virtiofs: Fixup printf args Fixup some fuse_log printf args for 32bit compatibility. Signed-off-by: Dr. David Alan Gilbert Message-Id: <20210428110100.27757-2-dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Stefan Hajnoczi --- tools/virtiofsd/passthrough_ll.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 2c36f4ec46..93a49db3cd 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -2011,10 +2011,10 @@ static void lo_getlk(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi, fuse_log(FUSE_LOG_DEBUG, "lo_getlk(ino=%" PRIu64 ", flags=%d)" - " owner=0x%lx, l_type=%d l_start=0x%lx" - " l_len=0x%lx\n", - ino, fi->flags, fi->lock_owner, lock->l_type, lock->l_start, - lock->l_len); + " owner=0x%" PRIx64 ", l_type=%d l_start=0x%" PRIx64 + " l_len=0x%" PRIx64 "\n", + ino, fi->flags, fi->lock_owner, lock->l_type, + (uint64_t)lock->l_start, (uint64_t)lock->l_len); if (!lo->posix_lock) { fuse_reply_err(req, ENOSYS); @@ -2061,10 +2061,10 @@ static void lo_setlk(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi, fuse_log(FUSE_LOG_DEBUG, "lo_setlk(ino=%" PRIu64 ", flags=%d)" - " cmd=%d pid=%d owner=0x%lx sleep=%d l_whence=%d" - " l_start=0x%lx l_len=0x%lx\n", + " cmd=%d pid=%d owner=0x%" PRIx64 " sleep=%d l_whence=%d" + " l_start=0x%" PRIx64 " l_len=0x%" PRIx64 "\n", ino, fi->flags, lock->l_type, lock->l_pid, fi->lock_owner, sleep, - lock->l_whence, lock->l_start, lock->l_len); + lock->l_whence, (uint64_t)lock->l_start, (uint64_t)lock->l_len); if (!lo->posix_lock) { fuse_reply_err(req, ENOSYS); @@ -3102,9 +3102,10 @@ static void lo_copy_file_range(fuse_req_t req, fuse_ino_t ino_in, off_t off_in, fuse_log(FUSE_LOG_DEBUG, "lo_copy_file_range(ino=%" PRIu64 "/fd=%d, " - "off=%lu, ino=%" PRIu64 "/fd=%d, " - "off=%lu, size=%zd, flags=0x%x)\n", - ino_in, in_fd, off_in, ino_out, out_fd, off_out, len, flags); + "off=%ju, ino=%" PRIu64 "/fd=%d, " + "off=%ju, size=%zd, flags=0x%x)\n", + ino_in, in_fd, (intmax_t)off_in, + ino_out, out_fd, (intmax_t)off_out, len, flags); res = copy_file_range(in_fd, &off_in, out_fd, &off_out, len, flags); if (res < 0) { -- cgit v1.2.3 From 5bf5188a116a4b7663662b1f19b2fe91ff0b5974 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Wed, 28 Apr 2021 12:00:36 +0100 Subject: virtiofsd: Don't assume header layout virtiofsd incorrectly assumed a fixed set of header layout in the virt queue; assuming that the fuse and write headers were conveniently separated from the data; the spec doesn't allow us to take that convenience, so fix it up to deal with it the hard way. Signed-off-by: Dr. David Alan Gilbert Message-Id: <20210428110100.27757-3-dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Stefan Hajnoczi --- tools/virtiofsd/fuse_virtio.c | 94 +++++++++++++++++++++++++++++++++---------- 1 file changed, 73 insertions(+), 21 deletions(-) diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c index 3e13997406..6dd73c9b72 100644 --- a/tools/virtiofsd/fuse_virtio.c +++ b/tools/virtiofsd/fuse_virtio.c @@ -129,18 +129,55 @@ static void fv_panic(VuDev *dev, const char *err) * Copy from an iovec into a fuse_buf (memory only) * Caller must ensure there is space */ -static void copy_from_iov(struct fuse_buf *buf, size_t out_num, - const struct iovec *out_sg) +static size_t copy_from_iov(struct fuse_buf *buf, size_t out_num, + const struct iovec *out_sg, + size_t max) { void *dest = buf->mem; + size_t copied = 0; - while (out_num) { + while (out_num && max) { size_t onelen = out_sg->iov_len; + onelen = MIN(onelen, max); memcpy(dest, out_sg->iov_base, onelen); dest += onelen; + copied += onelen; out_sg++; out_num--; + max -= onelen; } + + return copied; +} + +/* + * Skip 'skip' bytes in the iov; 'sg_1stindex' is set as + * the index for the 1st iovec to read data from, and + * 'sg_1stskip' is the number of bytes to skip in that entry. + * + * Returns True if there are at least 'skip' bytes in the iovec + * + */ +static bool skip_iov(const struct iovec *sg, size_t sg_size, + size_t skip, + size_t *sg_1stindex, size_t *sg_1stskip) +{ + size_t vec; + + for (vec = 0; vec < sg_size; vec++) { + if (sg[vec].iov_len > skip) { + *sg_1stskip = skip; + *sg_1stindex = vec; + + return true; + } + + skip -= sg[vec].iov_len; + } + + *sg_1stindex = vec; + *sg_1stskip = 0; + return skip == 0; } /* @@ -457,6 +494,7 @@ static void fv_queue_worker(gpointer data, gpointer user_data) bool allocated_bufv = false; struct fuse_bufvec bufv; struct fuse_bufvec *pbufv; + struct fuse_in_header inh; assert(se->bufsize > sizeof(struct fuse_in_header)); @@ -505,14 +543,15 @@ static void fv_queue_worker(gpointer data, gpointer user_data) elem->index); assert(0); /* TODO */ } - /* Copy just the first element and look at it */ - copy_from_iov(&fbuf, 1, out_sg); + /* Copy just the fuse_in_header and look at it */ + copy_from_iov(&fbuf, out_num, out_sg, + sizeof(struct fuse_in_header)); + memcpy(&inh, fbuf.mem, sizeof(struct fuse_in_header)); pbufv = NULL; /* Compiler thinks an unitialised path */ - if (out_num > 2 && - out_sg[0].iov_len == sizeof(struct fuse_in_header) && - ((struct fuse_in_header *)fbuf.mem)->opcode == FUSE_WRITE && - out_sg[1].iov_len == sizeof(struct fuse_write_in)) { + if (inh.opcode == FUSE_WRITE && + out_len >= (sizeof(struct fuse_in_header) + + sizeof(struct fuse_write_in))) { /* * For a write we don't actually need to copy the * data, we can just do it straight out of guest memory @@ -521,15 +560,15 @@ static void fv_queue_worker(gpointer data, gpointer user_data) */ fuse_log(FUSE_LOG_DEBUG, "%s: Write special case\n", __func__); - /* copy the fuse_write_in header afte rthe fuse_in_header */ - fbuf.mem += out_sg->iov_len; - copy_from_iov(&fbuf, 1, out_sg + 1); - fbuf.mem -= out_sg->iov_len; - fbuf.size = out_sg[0].iov_len + out_sg[1].iov_len; + fbuf.size = copy_from_iov(&fbuf, out_num, out_sg, + sizeof(struct fuse_in_header) + + sizeof(struct fuse_write_in)); + /* That copy reread the in_header, make sure we use the original */ + memcpy(fbuf.mem, &inh, sizeof(struct fuse_in_header)); /* Allocate the bufv, with space for the rest of the iov */ pbufv = malloc(sizeof(struct fuse_bufvec) + - sizeof(struct fuse_buf) * (out_num - 2)); + sizeof(struct fuse_buf) * out_num); if (!pbufv) { fuse_log(FUSE_LOG_ERR, "%s: pbufv malloc failed\n", __func__); @@ -540,24 +579,37 @@ static void fv_queue_worker(gpointer data, gpointer user_data) pbufv->count = 1; pbufv->buf[0] = fbuf; - size_t iovindex, pbufvindex; - iovindex = 2; /* 2 headers, separate iovs */ + size_t iovindex, pbufvindex, iov_bytes_skip; pbufvindex = 1; /* 2 headers, 1 fusebuf */ + if (!skip_iov(out_sg, out_num, + sizeof(struct fuse_in_header) + + sizeof(struct fuse_write_in), + &iovindex, &iov_bytes_skip)) { + fuse_log(FUSE_LOG_ERR, "%s: skip failed\n", + __func__); + goto out; + } + for (; iovindex < out_num; iovindex++, pbufvindex++) { pbufv->count++; pbufv->buf[pbufvindex].pos = ~0; /* Dummy */ pbufv->buf[pbufvindex].flags = 0; pbufv->buf[pbufvindex].mem = out_sg[iovindex].iov_base; pbufv->buf[pbufvindex].size = out_sg[iovindex].iov_len; + + if (iov_bytes_skip) { + pbufv->buf[pbufvindex].mem += iov_bytes_skip; + pbufv->buf[pbufvindex].size -= iov_bytes_skip; + iov_bytes_skip = 0; + } } } else { /* Normal (non fast write) path */ - /* Copy the rest of the buffer */ - fbuf.mem += out_sg->iov_len; - copy_from_iov(&fbuf, out_num - 1, out_sg + 1); - fbuf.mem -= out_sg->iov_len; + copy_from_iov(&fbuf, out_num, out_sg, se->bufsize); + /* That copy reread the in_header, make sure we use the original */ + memcpy(fbuf.mem, &inh, sizeof(struct fuse_in_header)); fbuf.size = out_len; /* TODO! Endianness of header */ -- cgit v1.2.3 From 98bbd186ed49ead7419ca9d9f2915c7cc1dd0083 Mon Sep 17 00:00:00 2001 From: Mahmoud Mandour Date: Tue, 20 Apr 2021 17:46:36 +0200 Subject: virtiofsd: Changed allocations of fuse_req to GLib functions Replaced the allocation and deallocation of fuse_req structs using calloc()/free() call pairs to a GLib's g_try_new0() and g_free(). Signed-off-by: Mahmoud Mandour Reviewed-by: Stefan Hajnoczi Message-Id: <20210420154643.58439-2-ma.mandourr@gmail.com> Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/fuse_lowlevel.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c index 58e32fc963..812cef6ef6 100644 --- a/tools/virtiofsd/fuse_lowlevel.c +++ b/tools/virtiofsd/fuse_lowlevel.c @@ -106,7 +106,7 @@ static void list_add_req(struct fuse_req *req, struct fuse_req *next) static void destroy_req(fuse_req_t req) { pthread_mutex_destroy(&req->lock); - free(req); + g_free(req); } void fuse_free_req(fuse_req_t req) @@ -130,7 +130,7 @@ static struct fuse_req *fuse_ll_alloc_req(struct fuse_session *se) { struct fuse_req *req; - req = (struct fuse_req *)calloc(1, sizeof(struct fuse_req)); + req = g_try_new0(struct fuse_req, 1); if (req == NULL) { fuse_log(FUSE_LOG_ERR, "fuse: failed to allocate request\n"); } else { @@ -1684,7 +1684,7 @@ static struct fuse_req *check_interrupt(struct fuse_session *se, if (curr->u.i.unique == req->unique) { req->interrupted = 1; list_del_req(curr); - free(curr); + g_free(curr); return NULL; } } -- cgit v1.2.3 From 01c6c6f982196458d60d4d7cfa963c38947949f5 Mon Sep 17 00:00:00 2001 From: Mahmoud Mandour Date: Tue, 27 Apr 2021 20:13:33 +0200 Subject: virtiofsd: Changed allocations of iovec to GLib's functions Replaced the calls to malloc()/calloc() and their respective calls to free() of iovec structs with GLib's allocation and deallocation functions and used g_autofree when appropriate. Replaced the allocation of in_sg_cpy to g_new() instead of a call to calloc() and a null-checking assertion. Not g_new0() because the buffer is immediately overwritten using memcpy. Signed-off-by: Mahmoud Mandour Message-Id: <20210427181333.148176-1-ma.mandourr@gmail.com> Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Dr. David Alan Gilbert --- tools/virtiofsd/fuse_lowlevel.c | 31 ++++++++++++------------------- tools/virtiofsd/fuse_virtio.c | 7 ++----- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c index 812cef6ef6..88496f9560 100644 --- a/tools/virtiofsd/fuse_lowlevel.c +++ b/tools/virtiofsd/fuse_lowlevel.c @@ -217,9 +217,9 @@ static int send_reply(fuse_req_t req, int error, const void *arg, int fuse_reply_iov(fuse_req_t req, const struct iovec *iov, int count) { int res; - struct iovec *padded_iov; + g_autofree struct iovec *padded_iov = NULL; - padded_iov = malloc((count + 1) * sizeof(struct iovec)); + padded_iov = g_try_new(struct iovec, count + 1); if (padded_iov == NULL) { return fuse_reply_err(req, ENOMEM); } @@ -228,7 +228,6 @@ int fuse_reply_iov(fuse_req_t req, const struct iovec *iov, int count) count++; res = send_reply_iov(req, 0, padded_iov, count); - free(padded_iov); return res; } @@ -568,7 +567,7 @@ static struct fuse_ioctl_iovec *fuse_ioctl_iovec_copy(const struct iovec *iov, struct fuse_ioctl_iovec *fiov; size_t i; - fiov = malloc(sizeof(fiov[0]) * count); + fiov = g_try_new(struct fuse_ioctl_iovec, count); if (!fiov) { return NULL; } @@ -586,8 +585,8 @@ int fuse_reply_ioctl_retry(fuse_req_t req, const struct iovec *in_iov, size_t out_count) { struct fuse_ioctl_out arg; - struct fuse_ioctl_iovec *in_fiov = NULL; - struct fuse_ioctl_iovec *out_fiov = NULL; + g_autofree struct fuse_ioctl_iovec *in_fiov = NULL; + g_autofree struct fuse_ioctl_iovec *out_fiov = NULL; struct iovec iov[4]; size_t count = 1; int res; @@ -603,13 +602,14 @@ int fuse_reply_ioctl_retry(fuse_req_t req, const struct iovec *in_iov, /* Can't handle non-compat 64bit ioctls on 32bit */ if (sizeof(void *) == 4 && req->ioctl_64bit) { res = fuse_reply_err(req, EINVAL); - goto out; + return res; } if (in_count) { in_fiov = fuse_ioctl_iovec_copy(in_iov, in_count); if (!in_fiov) { - goto enomem; + res = fuse_reply_err(req, ENOMEM); + return res; } iov[count].iov_base = (void *)in_fiov; @@ -619,7 +619,8 @@ int fuse_reply_ioctl_retry(fuse_req_t req, const struct iovec *in_iov, if (out_count) { out_fiov = fuse_ioctl_iovec_copy(out_iov, out_count); if (!out_fiov) { - goto enomem; + res = fuse_reply_err(req, ENOMEM); + return res; } iov[count].iov_base = (void *)out_fiov; @@ -628,15 +629,8 @@ int fuse_reply_ioctl_retry(fuse_req_t req, const struct iovec *in_iov, } res = send_reply_iov(req, 0, iov, count); -out: - free(in_fiov); - free(out_fiov); return res; - -enomem: - res = fuse_reply_err(req, ENOMEM); - goto out; } int fuse_reply_ioctl(fuse_req_t req, int result, const void *buf, size_t size) @@ -663,11 +657,11 @@ int fuse_reply_ioctl(fuse_req_t req, int result, const void *buf, size_t size) int fuse_reply_ioctl_iov(fuse_req_t req, int result, const struct iovec *iov, int count) { - struct iovec *padded_iov; + g_autofree struct iovec *padded_iov = NULL; struct fuse_ioctl_out arg; int res; - padded_iov = malloc((count + 2) * sizeof(struct iovec)); + padded_iov = g_try_new(struct iovec, count + 2); if (padded_iov == NULL) { return fuse_reply_err(req, ENOMEM); } @@ -680,7 +674,6 @@ int fuse_reply_ioctl_iov(fuse_req_t req, int result, const struct iovec *iov, memcpy(&padded_iov[2], iov, count * sizeof(struct iovec)); res = send_reply_iov(req, 0, padded_iov, count + 2); - free(padded_iov); return res; } diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c index 6dd73c9b72..a3d37ab696 100644 --- a/tools/virtiofsd/fuse_virtio.c +++ b/tools/virtiofsd/fuse_virtio.c @@ -331,6 +331,7 @@ int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch, VuVirtq *q = vu_get_queue(dev, qi->qidx); VuVirtqElement *elem = &req->elem; int ret = 0; + g_autofree struct iovec *in_sg_cpy = NULL; assert(count >= 1); assert(iov[0].iov_len >= sizeof(struct fuse_out_header)); @@ -384,8 +385,7 @@ int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch, * Build a copy of the the in_sg iov so we can skip bits in it, * including changing the offsets */ - struct iovec *in_sg_cpy = calloc(sizeof(struct iovec), in_num); - assert(in_sg_cpy); + in_sg_cpy = g_new(struct iovec, in_num); memcpy(in_sg_cpy, in_sg, sizeof(struct iovec) * in_num); /* These get updated as we skip */ struct iovec *in_sg_ptr = in_sg_cpy; @@ -423,7 +423,6 @@ int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch, ret = errno; fuse_log(FUSE_LOG_DEBUG, "%s: preadv failed (%m) len=%zd\n", __func__, len); - free(in_sg_cpy); goto err; } fuse_log(FUSE_LOG_DEBUG, "%s: preadv ret=%d len=%zd\n", __func__, @@ -447,13 +446,11 @@ int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch, if (ret != len) { fuse_log(FUSE_LOG_DEBUG, "%s: ret!=len\n", __func__); ret = EIO; - free(in_sg_cpy); goto err; } in_sg_left -= ret; len -= ret; } while (in_sg_left); - free(in_sg_cpy); /* Need to fix out->len on EOF */ if (len) { -- cgit v1.2.3 From f90a2d68c073c6a0a57790fae6b64eeb14ae78bb Mon Sep 17 00:00:00 2001 From: Mahmoud Mandour Date: Tue, 20 Apr 2021 17:46:38 +0200 Subject: virtiofsd: Changed allocations of fuse_session to GLib's functions Replaced the allocation and deallocation of fuse_session structs from calloc() and free() calls to g_try_new0() and g_free(). Signed-off-by: Mahmoud Mandour Reviewed-by: Stefan Hajnoczi Message-Id: <20210420154643.58439-4-ma.mandourr@gmail.com> Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/fuse_lowlevel.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c index 88496f9560..7fe2cef1eb 100644 --- a/tools/virtiofsd/fuse_lowlevel.c +++ b/tools/virtiofsd/fuse_lowlevel.c @@ -2470,7 +2470,7 @@ void fuse_session_destroy(struct fuse_session *se) free(se->vu_socket_path); se->vu_socket_path = NULL; - free(se); + g_free(se); } @@ -2493,7 +2493,7 @@ struct fuse_session *fuse_session_new(struct fuse_args *args, return NULL; } - se = (struct fuse_session *)calloc(1, sizeof(struct fuse_session)); + se = g_try_new0(struct fuse_session, 1); if (se == NULL) { fuse_log(FUSE_LOG_ERR, "fuse: failed to allocate fuse object\n"); goto out1; @@ -2553,7 +2553,7 @@ struct fuse_session *fuse_session_new(struct fuse_args *args, out4: fuse_opt_free_args(args); out2: - free(se); + g_free(se); out1: return NULL; } -- cgit v1.2.3 From e85d6d1ef2b950b55ec8b31b78335d320be13c0b Mon Sep 17 00:00:00 2001 From: Mahmoud Mandour Date: Tue, 20 Apr 2021 17:46:39 +0200 Subject: virtiofsd: Changed allocation of lo_map_elems to GLib's functions Replaced (re)allocation of lo_map_elem structs from realloc() to GLib's g_try_realloc_n() and replaced the respective free() call with a g_free(). Signed-off-by: Mahmoud Mandour Reviewed-by: Stefan Hajnoczi Message-Id: <20210420154643.58439-5-ma.mandourr@gmail.com> Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/passthrough_ll.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 93a49db3cd..406b5bd10e 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -406,7 +406,7 @@ static void lo_map_init(struct lo_map *map) static void lo_map_destroy(struct lo_map *map) { - free(map->elems); + g_free(map->elems); } static int lo_map_grow(struct lo_map *map, size_t new_nelems) @@ -418,7 +418,7 @@ static int lo_map_grow(struct lo_map *map, size_t new_nelems) return 1; } - new_elems = realloc(map->elems, sizeof(map->elems[0]) * new_nelems); + new_elems = g_try_realloc_n(map->elems, new_nelems, sizeof(map->elems[0])); if (!new_elems) { return 0; } -- cgit v1.2.3 From 31dfd22d7c8babae608eaf776d6d078fd6d7464f Mon Sep 17 00:00:00 2001 From: Mahmoud Mandour Date: Tue, 20 Apr 2021 17:46:40 +0200 Subject: virtiofsd: Changed allocations of fv_VuDev & its internals to GLib functions Changed the allocations of fv_VuDev structs, VuDev structs, and fv_QueueInfo strcuts from using calloc()/realloc() & free() to using the equivalent functions from GLib. In instances, removed the pair of allocation and assertion for non-NULL checking with a GLib function that aborts on error. Removed NULL-checking for fv_VuDev struct allocation and used a GLib function that crashes on error; namely, g_new0(). This is because allocating one struct should not be a problem on an healthy system. Also following the pattern of aborting-on-null behaviour that is taken with allocating VuDev structs and fv_QueueInfo structs. Signed-off-by: Mahmoud Mandour Reviewed-by: Stefan Hajnoczi Message-Id: <20210420154643.58439-6-ma.mandourr@gmail.com> Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/fuse_virtio.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c index a3d37ab696..828f0fa590 100644 --- a/tools/virtiofsd/fuse_virtio.c +++ b/tools/virtiofsd/fuse_virtio.c @@ -782,7 +782,7 @@ static void fv_queue_cleanup_thread(struct fv_VuDev *vud, int qidx) pthread_mutex_destroy(&ourqi->vq_lock); close(ourqi->kill_fd); ourqi->kick_fd = -1; - free(vud->qi[qidx]); + g_free(vud->qi[qidx]); vud->qi[qidx] = NULL; } @@ -813,15 +813,13 @@ static void fv_queue_set_started(VuDev *dev, int qidx, bool started) if (started) { /* Fire up a thread to watch this queue */ if (qidx >= vud->nqueues) { - vud->qi = realloc(vud->qi, (qidx + 1) * sizeof(vud->qi[0])); - assert(vud->qi); + vud->qi = g_realloc_n(vud->qi, qidx + 1, sizeof(vud->qi[0])); memset(vud->qi + vud->nqueues, 0, sizeof(vud->qi[0]) * (1 + (qidx - vud->nqueues))); vud->nqueues = qidx + 1; } if (!vud->qi[qidx]) { - vud->qi[qidx] = calloc(sizeof(struct fv_QueueInfo), 1); - assert(vud->qi[qidx]); + vud->qi[qidx] = g_new0(struct fv_QueueInfo, 1); vud->qi[qidx]->virtio_dev = vud; vud->qi[qidx]->qidx = qidx; } else { @@ -1087,12 +1085,7 @@ int virtio_session_mount(struct fuse_session *se) __func__); /* TODO: Some cleanup/deallocation! */ - se->virtio_dev = calloc(sizeof(struct fv_VuDev), 1); - if (!se->virtio_dev) { - fuse_log(FUSE_LOG_ERR, "%s: virtio_dev calloc failed\n", __func__); - close(data_sock); - return -1; - } + se->virtio_dev = g_new0(struct fv_VuDev, 1); se->vu_socketfd = data_sock; se->virtio_dev->se = se; @@ -1114,8 +1107,8 @@ void virtio_session_close(struct fuse_session *se) return; } - free(se->virtio_dev->qi); + g_free(se->virtio_dev->qi); pthread_rwlock_destroy(&se->virtio_dev->vu_dispatch_rwlock); - free(se->virtio_dev); + g_free(se->virtio_dev); se->virtio_dev = NULL; } -- cgit v1.2.3 From c9a276f57cd59ead43a62f0662b3d18f5072641b Mon Sep 17 00:00:00 2001 From: Mahmoud Mandour Date: Tue, 20 Apr 2021 17:46:41 +0200 Subject: virtiofsd/passthrough_ll.c: Changed local allocations to GLib functions Changed the allocations of some local variables to GLib's allocation functions, such as g_try_malloc0(), and annotated those variables as g_autofree. Subsequently, I was able to remove the calls to free(). Signed-off-by: Mahmoud Mandour Reviewed-by: Stefan Hajnoczi Message-Id: <20210420154643.58439-7-ma.mandourr@gmail.com> Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/passthrough_ll.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 406b5bd10e..49c21fd855 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -1653,7 +1653,7 @@ static void lo_do_readdir(fuse_req_t req, fuse_ino_t ino, size_t size, struct lo_data *lo = lo_data(req); struct lo_dirp *d = NULL; struct lo_inode *dinode; - char *buf = NULL; + g_autofree char *buf = NULL; char *p; size_t rem = size; int err = EBADF; @@ -1669,7 +1669,7 @@ static void lo_do_readdir(fuse_req_t req, fuse_ino_t ino, size_t size, } err = ENOMEM; - buf = calloc(1, size); + buf = g_try_malloc0(size); if (!buf) { goto error; } @@ -1755,7 +1755,6 @@ error: } else { fuse_reply_buf(req, buf, size - rem); } - free(buf); } static void lo_readdir(fuse_req_t req, fuse_ino_t ino, size_t size, @@ -2732,7 +2731,7 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, size_t size) { struct lo_data *lo = lo_data(req); - char *value = NULL; + g_autofree char *value = NULL; char procname[64]; const char *name; char *mapped_name; @@ -2773,7 +2772,7 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, ino, name, size); if (size) { - value = malloc(size); + value = g_try_malloc(size); if (!value) { goto out_err; } @@ -2812,8 +2811,6 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name, fuse_reply_xattr(req, ret); } out_free: - free(value); - if (fd >= 0) { close(fd); } @@ -2832,7 +2829,7 @@ 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; + g_autofree char *value = NULL; char procname[64]; struct lo_inode *inode; ssize_t ret; @@ -2854,7 +2851,7 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) size); if (size) { - value = malloc(size); + value = g_try_malloc(size); if (!value) { goto out_err; } @@ -2939,8 +2936,6 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) fuse_reply_xattr(req, ret); } out_free: - free(value); - if (fd >= 0) { close(fd); } -- cgit v1.2.3 From 67a010f64cc9e33ba19ab389dedaa52013a9de8a Mon Sep 17 00:00:00 2001 From: Mahmoud Mandour Date: Tue, 20 Apr 2021 17:46:42 +0200 Subject: virtiofsd/fuse_virtio.c: Changed allocations of locals to GLib Replaced the allocation of local variables from malloc() to GLib allocation functions. In one instance, dropped the usage to an assert after a malloc() call and used g_malloc() instead. Signed-off-by: Mahmoud Mandour Reviewed-by: Stefan Hajnoczi Message-Id: <20210420154643.58439-8-ma.mandourr@gmail.com> Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/fuse_virtio.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c index 828f0fa590..1170f375a5 100644 --- a/tools/virtiofsd/fuse_virtio.c +++ b/tools/virtiofsd/fuse_virtio.c @@ -511,8 +511,7 @@ static void fv_queue_worker(gpointer data, gpointer user_data) * They're spread over multiple descriptors in a scatter/gather set * and we can't trust the guest to keep them still; so copy in/out. */ - fbuf.mem = malloc(se->bufsize); - assert(fbuf.mem); + fbuf.mem = g_malloc(se->bufsize); fuse_mutex_init(&req->ch.lock); req->ch.fd = -1; @@ -564,8 +563,8 @@ static void fv_queue_worker(gpointer data, gpointer user_data) memcpy(fbuf.mem, &inh, sizeof(struct fuse_in_header)); /* Allocate the bufv, with space for the rest of the iov */ - pbufv = malloc(sizeof(struct fuse_bufvec) + - sizeof(struct fuse_buf) * out_num); + pbufv = g_try_malloc(sizeof(struct fuse_bufvec) + + sizeof(struct fuse_buf) * out_num); if (!pbufv) { fuse_log(FUSE_LOG_ERR, "%s: pbufv malloc failed\n", __func__); @@ -622,7 +621,7 @@ static void fv_queue_worker(gpointer data, gpointer user_data) out: if (allocated_bufv) { - free(pbufv); + g_free(pbufv); } /* If the request has no reply, still recycle the virtqueue element */ @@ -641,7 +640,7 @@ out: } pthread_mutex_destroy(&req->ch.lock); - free(fbuf.mem); + g_free(fbuf.mem); free(req); } -- cgit v1.2.3