From 38bccfbeb0af039e59eb75fe6d9b2a83cda3d381 Mon Sep 17 00:00:00 2001 From: Zach van Rijn Date: Wed, 1 Apr 2020 16:30:48 -0500 Subject: [PATCH 1/6] um: Add include: memset() and memcpy() are in These two functions are otherwise unknown to the pedantic compiler. Include the correct header to enable the build to succeed. Signed-off-by: Zach van Rijn Acked-By: Anton Ivanov Signed-off-by: Richard Weinberger --- arch/um/os-Linux/file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c index 26ecbd64c409..044836ad7392 100644 --- a/arch/um/os-Linux/file.c +++ b/arch/um/os-Linux/file.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include From bc8f8e4e6e7a648e8c6357307d614be8fdcfdf2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 7 Apr 2020 22:28:53 +0200 Subject: [PATCH 2/6] um: Add a generic "fd" vector transport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Learn to take a pre-opened file-descriptor for vector IO. Instead of teaching the driver to open a FD in multiple ways, it can rely on management layer to do it on its behalf. For example, this allows inheriting a preconfigured device fd or a simple socketpair() setup, without further arguments, privileges or system access by UML. Signed-off-by: Marc-André Lureau Acked-By: Anton Ivanov Signed-off-by: Richard Weinberger --- arch/um/drivers/vector_user.c | 59 +++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/arch/um/drivers/vector_user.c b/arch/um/drivers/vector_user.c index aa28e9eecb7b..c4a0f26b2824 100644 --- a/arch/um/drivers/vector_user.c +++ b/arch/um/drivers/vector_user.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include "vector_user.h" @@ -42,6 +43,9 @@ #define TRANS_RAW "raw" #define TRANS_RAW_LEN strlen(TRANS_RAW) +#define TRANS_FD "fd" +#define TRANS_FD_LEN strlen(TRANS_FD) + #define VNET_HDR_FAIL "could not enable vnet headers on fd %d" #define TUN_GET_F_FAIL "tapraw: TUNGETFEATURES failed: %s" #define L2TPV3_BIND_FAIL "l2tpv3_open : could not bind socket err=%i" @@ -347,6 +351,59 @@ unix_cleanup: return NULL; } +static int strtofd(const char *nptr) +{ + long fd; + char *endptr; + + if (nptr == NULL) + return -1; + + errno = 0; + fd = strtol(nptr, &endptr, 10); + if (nptr == endptr || + errno != 0 || + *endptr != '\0' || + fd < 0 || + fd > INT_MAX) { + return -1; + } + return fd; +} + +static struct vector_fds *user_init_fd_fds(struct arglist *ifspec) +{ + int fd = -1; + char *fdarg = NULL; + struct vector_fds *result = NULL; + + fdarg = uml_vector_fetch_arg(ifspec, "fd"); + fd = strtofd(fdarg); + if (fd == -1) { + printk(UM_KERN_ERR "fd open: bad or missing fd argument"); + goto fd_cleanup; + } + + result = uml_kmalloc(sizeof(struct vector_fds), UM_GFP_KERNEL); + if (result == NULL) { + printk(UM_KERN_ERR "fd open: allocation failed"); + goto fd_cleanup; + } + + result->rx_fd = fd; + result->tx_fd = fd; + result->remote_addr_size = 0; + result->remote_addr = NULL; + return result; + +fd_cleanup: + if (fd >= 0) + os_close_file(fd); + if (result != NULL) + kfree(result); + return NULL; +} + static struct vector_fds *user_init_raw_fds(struct arglist *ifspec) { int rxfd = -1, txfd = -1; @@ -578,6 +635,8 @@ struct vector_fds *uml_vector_user_open( return user_init_socket_fds(parsed, ID_L2TPV3); if (strncmp(transport, TRANS_BESS, TRANS_BESS_LEN) == 0) return user_init_unix_fds(parsed, ID_BESS); + if (strncmp(transport, TRANS_FD, TRANS_FD_LEN) == 0) + return user_init_fd_fds(parsed); return NULL; } From 4c5a770580054c1d107ba204a42df1db221670bd Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Sat, 11 Apr 2020 09:28:08 -0700 Subject: [PATCH 3/6] um: Neaten vu_err macro definition Defining a macro with ... and __VA_ARGS__ (without ##) can cause compilation errors if a macro use does not have additional args. Add ## to __VA_ARGS__ in the macro definition. Signed-off-by: Joe Perches Signed-off-by: Richard Weinberger --- arch/um/drivers/virtio_uml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c index be54d368e73d..351aee52aca6 100644 --- a/arch/um/drivers/virtio_uml.c +++ b/arch/um/drivers/virtio_uml.c @@ -74,7 +74,7 @@ struct virtio_uml_vq_info { extern unsigned long long physmem_size, highmem; -#define vu_err(vu_dev, ...) dev_err(&(vu_dev)->pdev->dev, __VA_ARGS__) +#define vu_err(vu_dev, ...) dev_err(&(vu_dev)->pdev->dev, ##__VA_ARGS__) /* Vhost-user protocol */ From 0b86ce29cfc273501d019b18f31cee7837c8fd2a Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Sat, 18 Apr 2020 03:04:55 +0900 Subject: [PATCH 4/6] um: Do not evaluate compiler's library path when cleaning Since commit a83e4ca26af8 ("kbuild: remove cc-option switch from -Wframe-larger-than="), 'make ARCH=um clean' emits an error message as follows: $ make ARCH=um clean gcc: error: missing argument to '-Wframe-larger-than=' We do not care compiler flags when cleaning. Use the '=' operator for lazy expansion because we do not use LDFLAGS_pcap.o or LDFLAGS_vde.o when cleaning. While I was here, I removed the redundant -r option because it already exists in the recipe. Fixes: a83e4ca26af8 ("kbuild: remove cc-option switch from -Wframe-larger-than=") Signed-off-by: Masahiro Yamada Reviewed-by: Nathan Chancellor Tested-by: Nathan Chancellor [build] Signed-off-by: Richard Weinberger --- arch/um/drivers/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/um/drivers/Makefile b/arch/um/drivers/Makefile index a290821e355c..2a249f619467 100644 --- a/arch/um/drivers/Makefile +++ b/arch/um/drivers/Makefile @@ -18,9 +18,9 @@ ubd-objs := ubd_kern.o ubd_user.o port-objs := port_kern.o port_user.o harddog-objs := harddog_kern.o harddog_user.o -LDFLAGS_pcap.o := -r $(shell $(CC) $(KBUILD_CFLAGS) -print-file-name=libpcap.a) +LDFLAGS_pcap.o = $(shell $(CC) $(KBUILD_CFLAGS) -print-file-name=libpcap.a) -LDFLAGS_vde.o := -r $(shell $(CC) $(CFLAGS) -print-file-name=libvdeplug.a) +LDFLAGS_vde.o = $(shell $(CC) $(CFLAGS) -print-file-name=libvdeplug.a) targets := pcap_kern.o pcap_user.o vde_kern.o vde_user.o From 54ebe4060fe6c4ab76c79354417612ac9cf4f95e Mon Sep 17 00:00:00 2001 From: Anton Ivanov Date: Wed, 22 Apr 2020 17:00:01 +0100 Subject: [PATCH 5/6] um: Use fdatasync() when mapping the UBD FSYNC command We do not need to update the metadata (atime, mtime, etc) on the UBD file and/or the COW file until UML exits. UBD image mtime is checked in UML only when opening the files. After that they are locked and used exclusively by a single UML instance, so there is no point wasting resources on updating metadata on every sync. We can sync data only. The host will always update mtime if a file has been modified upon closing it. Signed-off-by: Anton Ivanov Signed-off-by: Richard Weinberger --- arch/um/os-Linux/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c index 044836ad7392..e4421dbc4c36 100644 --- a/arch/um/os-Linux/file.c +++ b/arch/um/os-Linux/file.c @@ -290,7 +290,7 @@ int os_write_file(int fd, const void *buf, int len) int os_sync_file(int fd) { - int n = fsync(fd); + int n = fdatasync(fd); if (n < 0) return -errno; From f6e8c474390be2e6f5bd0b8966e19958214609ff Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 7 May 2020 14:26:52 -0500 Subject: [PATCH 6/6] um: virtio: Replace zero-length array with flexible-array The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99: struct foo { int stuff; struct boo array[]; }; By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertently introduced[3] to the codebase from now on. Also, notice that, dynamic memory allocations won't be affected by this change: "Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1] sizeof(flexible-array-member) triggers a warning because flexible array members have incomplete type[1]. There are some instances of code in which the sizeof operator is being incorrectly/erroneously applied to zero-length arrays and the result is zero. Such instances may be hiding some bugs. So, this work (flexible-array member conversions) will also help to get completely rid of those sorts of issues. This issue was found with the help of Coccinelle. [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html [2] https://github.com/KSPP/linux/issues/21 [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") Signed-off-by: Gustavo A. R. Silva Signed-off-by: Richard Weinberger --- arch/um/drivers/vector_kern.h | 2 +- arch/um/drivers/vhost_user.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/um/drivers/vector_kern.h b/arch/um/drivers/vector_kern.h index d0159082faf0..8fff93a75a92 100644 --- a/arch/um/drivers/vector_kern.h +++ b/arch/um/drivers/vector_kern.h @@ -129,7 +129,7 @@ struct vector_private { struct vector_estats estats; struct sock_fprog *bpf; - char user[0]; + char user[]; }; extern int build_transport_data(struct vector_private *vp); diff --git a/arch/um/drivers/vhost_user.h b/arch/um/drivers/vhost_user.h index 6c71b6005177..6f147cd3c9f7 100644 --- a/arch/um/drivers/vhost_user.h +++ b/arch/um/drivers/vhost_user.h @@ -78,7 +78,7 @@ struct vhost_user_config { u32 offset; u32 size; u32 flags; - u8 payload[0]; /* Variable length */ + u8 payload[]; /* Variable length */ } __packed; struct vhost_user_vring_state {