From c2bc3fd6daebeeeb7a63887e0582ae7bc92b44f0 Mon Sep 17 00:00:00 2001 From: Paul Sokolovsky Date: Tue, 19 May 2020 12:47:29 +0300 Subject: [PATCH 1/5] lib: os: fdtable: Add syscalls for POSIX read/write/close operations Fdtable is a kernel-level structure, so functions accessing it, like read(), write(), close(), should be syscalls. This conversion however emphasizes a difference between "syscall" and "C standard library function". C stdlib functions should be represented by just a standard linkage symbol, and can be used even without prototype. Syscall is however a special code sequence, which may require special attributes and thus a prototype for usage. To deal with these distinction, we define a convention that "sys_name" is a syscall, while "name" is a normal C function (implemented as a wrapper around syscall). (And we also need to define "_name" to interface with Newlib (actually, just "_write".)) Fixes: #25407 Signed-off-by: Paul Sokolovsky --- include/posix/unistd.h | 14 +++++++++- lib/os/fdtable.c | 61 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/include/posix/unistd.h b/include/posix/unistd.h index e9c955c5bf0d1..3765ad4381ae3 100644 --- a/include/posix/unistd.h +++ b/include/posix/unistd.h @@ -22,7 +22,15 @@ extern "C" { #endif #ifdef CONFIG_POSIX_API -/* File related operations */ +/* File related operations. Convention: "sys_name" is a syscall (needs + * prototype in this file for usage). "name" is a normal userspace + * function (implemented as a wrapper for syscall), usable even + * without prototype, per classical C handling. This distinction + * is however implemented on demand, based on the actual usecases seen. + */ +__syscall int sys_close(int file); +__syscall ssize_t sys_write(int file, const void *buffer, size_t count); +__syscall ssize_t sys_read(int file, void *buffer, size_t count); extern int close(int file); extern ssize_t write(int file, const void *buffer, size_t count); extern ssize_t read(int file, void *buffer, size_t count); @@ -50,4 +58,8 @@ int usleep(useconds_t useconds); } #endif +#ifndef CONFIG_ARCH_POSIX +#include +#endif /* CONFIG_ARCH_POSIX */ + #endif /* ZEPHYR_INCLUDE_POSIX_UNISTD_H_ */ diff --git a/lib/os/fdtable.c b/lib/os/fdtable.c index 53dcbecf15379..4efbcb017221e 100644 --- a/lib/os/fdtable.c +++ b/lib/os/fdtable.c @@ -18,6 +18,9 @@ #include #include #include +#ifdef CONFIG_POSIX_API +#include +#endif #include struct fd_entry { @@ -167,7 +170,7 @@ int z_alloc_fd(void *obj, const struct fd_op_vtable *vtable) #ifdef CONFIG_POSIX_API -ssize_t read(int fd, void *buf, size_t sz) +ssize_t z_impl_sys_read(int fd, void *buf, size_t sz) { if (_check_fd(fd) < 0) { return -1; @@ -175,9 +178,30 @@ ssize_t read(int fd, void *buf, size_t sz) return fdtable[fd].vtable->read(fdtable[fd].obj, buf, sz); } + +#ifdef CONFIG_USERSPACE +ssize_t z_vrfy_sys_read(int fd, void *buf, size_t sz) +{ + if (Z_SYSCALL_MEMORY_WRITE(buf, sz)) { + errno = EFAULT; + return -1; + } + + return z_impl_sys_read(fd, buf, sz); +} +#include +#endif /* CONFIG_USERSPACE */ + +/* Normal C function wrapping a corresponding syscall. Required to ensure + * classic C linkage. + */ +ssize_t read(int fd, void *buf, size_t sz) +{ + return sys_read(fd, buf, sz); +} FUNC_ALIAS(read, _read, ssize_t); -ssize_t write(int fd, const void *buf, size_t sz) +ssize_t z_impl_sys_write(int fd, const void *buf, size_t sz) { if (_check_fd(fd) < 0) { return -1; @@ -185,9 +209,27 @@ ssize_t write(int fd, const void *buf, size_t sz) return fdtable[fd].vtable->write(fdtable[fd].obj, buf, sz); } + +#ifdef CONFIG_USERSPACE +ssize_t z_vrfy_sys_write(int fd, const void *buf, size_t sz) +{ + Z_OOPS(Z_SYSCALL_MEMORY_READ(buf, sz)); + + return z_impl_sys_write(fd, buf, sz); +} +#include +#endif /* CONFIG_USERSPACE */ + +/* Normal C function wrapping a corresponding syscall. Required to ensure + * classic C linkage. + */ +ssize_t write(int fd, const void *buf, size_t sz) +{ + return sys_write(fd, buf, sz); +} FUNC_ALIAS(write, _write, ssize_t); -int close(int fd) +int z_impl_sys_close(int fd) { int res; @@ -200,6 +242,19 @@ int close(int fd) return res; } + +#ifdef CONFIG_USERSPACE +ssize_t z_vrfy_sys_close(int fd) +{ + return z_impl_sys_close(fd); +} +#include +#endif /* CONFIG_USERSPACE */ + +int close(int fd) +{ + return sys_close(fd); +} FUNC_ALIAS(close, _close, int); int fsync(int fd) From 45f485f354a5c10a5d2c4b5adc67f1123278402c Mon Sep 17 00:00:00 2001 From: Paul Sokolovsky Date: Wed, 20 May 2020 11:52:45 +0300 Subject: [PATCH 2/5] lib: os: fdtable: Add syscall for POSIX ioctl operation This is similar to previously added syscalls for read/write/close. sys_ioctl accepts va_list. fcntl() is clearly rewritten as a userspace operation, delegating to sys_ioctl(). Signed-off-by: Paul Sokolovsky --- include/posix/sys/ioctl.h | 9 ++++++++ include/sys/fdtable.h | 9 ++++++-- lib/os/fdtable.c | 46 ++++++++++++++++++++++++++++++--------- 3 files changed, 52 insertions(+), 12 deletions(-) diff --git a/include/posix/sys/ioctl.h b/include/posix/sys/ioctl.h index 35e046ab8d26b..87d71fea5f938 100644 --- a/include/posix/sys/ioctl.h +++ b/include/posix/sys/ioctl.h @@ -6,6 +6,15 @@ #ifndef ZEPHYR_INCLUDE_POSIX_SYS_IOCTL_H_ #define ZEPHYR_INCLUDE_POSIX_SYS_IOCTL_H_ +#include + +__syscall int sys_ioctl(int fd, unsigned long request, va_list args); +int ioctl(int fd, unsigned long request, ...); + #define FIONBIO 0x5421 +#ifndef CONFIG_ARCH_POSIX +#include +#endif /* CONFIG_ARCH_POSIX */ + #endif /* ZEPHYR_INCLUDE_POSIX_SYS_IOCTL_H_ */ diff --git a/include/sys/fdtable.h b/include/sys/fdtable.h index 0cd4b2513d032..3f1aada23858d 100644 --- a/include/sys/fdtable.h +++ b/include/sys/fdtable.h @@ -138,10 +138,15 @@ enum { ZFD_IOCTL_CLOSE = 0x100, ZFD_IOCTL_FSYNC, ZFD_IOCTL_LSEEK, - ZFD_IOCTL_POLL_PREPARE, + ZFD_IOCTL_GETSOCKNAME, + + /* Codes above 0xff00 are private kernel-only requests, not + * available from userspace. + */ + ZFD_IOCTL_PRIVATE = 0xff00, + ZFD_IOCTL_POLL_PREPARE = ZFD_IOCTL_PRIVATE, ZFD_IOCTL_POLL_UPDATE, ZFD_IOCTL_POLL_OFFLOAD, - ZFD_IOCTL_GETSOCKNAME, }; #ifdef __cplusplus diff --git a/lib/os/fdtable.c b/lib/os/fdtable.c index 4efbcb017221e..e3055adfafd68 100644 --- a/lib/os/fdtable.c +++ b/lib/os/fdtable.c @@ -20,6 +20,7 @@ #include #ifdef CONFIG_POSIX_API #include +#include #endif #include @@ -277,17 +278,46 @@ off_t lseek(int fd, off_t offset, int whence) } FUNC_ALIAS(lseek, _lseek, off_t); -int ioctl(int fd, unsigned long request, ...) +int z_impl_sys_ioctl(int fd, unsigned long request, va_list args) { - va_list args; - int res; - if (_check_fd(fd) < 0) { return -1; } + return fdtable[fd].vtable->ioctl(fdtable[fd].obj, request, args); +} + +#ifdef CONFIG_USERSPACE +ssize_t z_vrfy_sys_ioctl(int fd, unsigned long request, va_list args) +{ +/* This doesn't work so far, until we start to fish inside a particular + * va_list structure for a particular arch. Maybe later. + */ +#if 0 + /* Check that we can access 4 words of arguments. This is very + * generic check, implementation of specific ioctl's are + * expected to perform their own detailed argument checking. + */ + Z_OOPS(Z_SYSCALL_MEMORY_READ(args, sizeof(uintptr_t) * 4)); +#endif + + if (request >= ZFD_IOCTL_PRIVATE) { + errno = EINVAL; + return -1; + } + + return z_impl_sys_ioctl(fd, request, args); +} +#include +#endif /* CONFIG_USERSPACE */ + +int ioctl(int fd, unsigned long request, ...) +{ + va_list args; + int res; + va_start(args, request); - res = fdtable[fd].vtable->ioctl(fdtable[fd].obj, request, args); + res = sys_ioctl(fd, request, args); va_end(args); return res; @@ -304,10 +334,6 @@ int fcntl(int fd, int cmd, ...) va_list args; int res; - if (_check_fd(fd) < 0) { - return -1; - } - /* Handle fdtable commands. */ switch (cmd) { case F_DUPFD: @@ -318,7 +344,7 @@ int fcntl(int fd, int cmd, ...) /* The rest of commands are per-fd, handled by ioctl vmethod. */ va_start(args, cmd); - res = fdtable[fd].vtable->ioctl(fdtable[fd].obj, cmd, args); + res = sys_ioctl(fd, cmd, args); va_end(args); return res; From e9944b9822f4d85b72775f85e8feac435108f28b Mon Sep 17 00:00:00 2001 From: Paul Sokolovsky Date: Wed, 20 May 2020 15:41:11 +0300 Subject: [PATCH 3/5] tests: net: socket: tcp: Add testcase for write()/read(). Just a test for send()/recv() duplicated to call write()/read() instead. Duplication is not ideal, but to deal with it, we would need some no-nonsense metagramming capabilities. Sadly, C classically have problems with that. Standard way to instantiate different functions based on the same template is to use preprecessor, but that requires suffixing each line with a backslash, and thus is pretty unwieldy. A more modern approach would be to use always-inline function as a template, but always-inline functions aren't part of the C standard, and just inline functions don't guarantee inlinability. (Besides metaprogramming, a mundane-programming solution is possible, but somehow that was considered cumersome too.) Signed-off-by: Paul Sokolovsky --- tests/net/socket/socket_helpers.h | 5 +++ tests/net/socket/tcp/src/main.c | 72 ++++++++++++++++++++++++++++++ tests/net/socket/tcp/testcase.yaml | 9 +++- 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/tests/net/socket/socket_helpers.h b/tests/net/socket/socket_helpers.h index 69bb20c14b02a..e8bea92e3321b 100644 --- a/tests/net/socket/socket_helpers.h +++ b/tests/net/socket/socket_helpers.h @@ -6,7 +6,12 @@ #include +#ifdef CONFIG_POSIX_API +#include +#include +#else #include +#endif #define clear_buf(buf) memset(buf, 0, sizeof(buf)) diff --git a/tests/net/socket/tcp/src/main.c b/tests/net/socket/tcp/src/main.c index aa840c2168217..a4bb62ae015cf 100644 --- a/tests/net/socket/tcp/src/main.c +++ b/tests/net/socket/tcp/src/main.c @@ -11,6 +11,10 @@ LOG_MODULE_REGISTER(net_test, CONFIG_NET_SOCKETS_LOG_LEVEL); #include #include +#ifdef CONFIG_POSIX_API +#include +#endif + #include "../../socket_helpers.h" #define TEST_STR_SMALL "test" @@ -50,6 +54,15 @@ static void test_send(int sock, const void *buf, size_t len, int flags) "send failed"); } +#ifdef CONFIG_POSIX_API +static void test_write(int sock, const void *buf, size_t len) +{ + zassert_equal(write(sock, buf, len), + len, + "write failed"); +} +#endif + static void test_sendto(int sock, const void *buf, size_t len, int flags, const struct sockaddr *addr, socklen_t addrlen) { @@ -96,6 +109,22 @@ static void test_recv(int sock, int flags) "unexpected data"); } +#ifdef CONFIG_POSIX_API +static void test_read(int sock) +{ + ssize_t recved = 0; + char rx_buf[30] = {0}; + + recved = read(sock, rx_buf, sizeof(rx_buf)); + zassert_equal(recved, + strlen(TEST_STR_SMALL), + "unexpected received bytes"); + zassert_equal(strncmp(rx_buf, TEST_STR_SMALL, strlen(TEST_STR_SMALL)), + 0, + "unexpected data"); +} +#endif + static void test_recvfrom(int sock, int flags, struct sockaddr *addr, @@ -147,6 +176,7 @@ static void test_eof(int sock) zassert_equal(recved, 0, ""); } +/* Keep in sync with test_v4_write_read() below. */ void test_v4_send_recv(void) { /* Test if send() and recv() work on a ipv4 stream socket. */ @@ -184,6 +214,47 @@ void test_v4_send_recv(void) k_sleep(TCP_TEARDOWN_TIMEOUT); } +/* Keep in sync with test_v4_send_recv() above. */ +void test_v4_write_read(void) +{ +#ifndef CONFIG_POSIX_API + ztest_test_skip(); +#else + /* Test if send() and recv() work on a ipv4 stream socket. */ + int c_sock; + int s_sock; + int new_sock; + struct sockaddr_in c_saddr; + struct sockaddr_in s_saddr; + struct sockaddr addr; + socklen_t addrlen = sizeof(addr); + + prepare_sock_tcp_v4(CONFIG_NET_CONFIG_MY_IPV4_ADDR, ANY_PORT, + &c_sock, &c_saddr); + prepare_sock_tcp_v4(CONFIG_NET_CONFIG_MY_IPV4_ADDR, SERVER_PORT, + &s_sock, &s_saddr); + + test_bind(s_sock, (struct sockaddr *)&s_saddr, sizeof(s_saddr)); + test_listen(s_sock); + + test_connect(c_sock, (struct sockaddr *)&s_saddr, sizeof(s_saddr)); + test_write(c_sock, TEST_STR_SMALL, strlen(TEST_STR_SMALL)); + + test_accept(s_sock, &new_sock, &addr, &addrlen); + zassert_equal(addrlen, sizeof(struct sockaddr_in), "wrong addrlen"); + + test_read(new_sock); + + test_close(c_sock); + test_eof(new_sock); + + test_close(new_sock); + test_close(s_sock); + + k_sleep(TCP_TEARDOWN_TIMEOUT); +#endif +} + void test_v6_send_recv(void) { /* Test if send() and recv() work on a ipv6 stream socket. */ @@ -516,6 +587,7 @@ void test_main(void) ztest_test_suite( socket_tcp, ztest_user_unit_test(test_v4_send_recv), + ztest_user_unit_test(test_v4_write_read), ztest_user_unit_test(test_v6_send_recv), ztest_user_unit_test(test_v4_sendto_recvfrom), ztest_user_unit_test(test_v6_sendto_recvfrom), diff --git a/tests/net/socket/tcp/testcase.yaml b/tests/net/socket/tcp/testcase.yaml index 8c64053dbc030..7661b813b120b 100644 --- a/tests/net/socket/tcp/testcase.yaml +++ b/tests/net/socket/tcp/testcase.yaml @@ -1,6 +1,11 @@ common: depends_on: netif + min_ram: 32 + tags: net socket userspace tests: net.socket.tcp: - min_ram: 32 - tags: net socket userspace + extra_configs: + - CONFIG_POSIX_API=n + net.socket.tcp.posix: + extra_configs: + - CONFIG_POSIX_API=y From 3e15b383cd338c2ceb7c57df435728ff02da25e7 Mon Sep 17 00:00:00 2001 From: Paul Sokolovsky Date: Fri, 22 May 2020 16:00:31 +0300 Subject: [PATCH 4/5] lib: os: fdtable: Make ioctl method accept array of args instead of va_list As we cannot pass va_list across userspace/kernel boundary (because we don't control its internel representation, and it may be quite, complex), just get rid of it on the lowest level of ioctl machinery. Pass just explicit array of machine words (uintptr_t) to the ioctl vmethod implementation. On the API convenience layer, there's still support for variadic arguments calls (both on the level of userspace ioctl() function, and for internal in-kernel usage), with explicit marshalling from va_list to array of uintptr_t's. Signed-off-by: Paul Sokolovsky --- include/posix/sys/ioctl.h | 2 +- include/sys/fdtable.h | 23 ++++--- lib/os/fdtable.c | 80 +++++++++++++++++------ lib/posix/eventfd.c | 15 +++-- lib/posix/fs.c | 7 +- subsys/net/lib/sockets/socketpair.c | 15 +++-- subsys/net/lib/sockets/sockets_can.c | 5 +- subsys/net/lib/sockets/sockets_net_mgmt.c | 4 +- subsys/net/lib/sockets/sockets_packet.c | 6 +- subsys/net/lib/sockets/sockets_tls.c | 16 +++-- subsys/net/lib/websocket/websocket.c | 5 +- 11 files changed, 115 insertions(+), 63 deletions(-) diff --git a/include/posix/sys/ioctl.h b/include/posix/sys/ioctl.h index 87d71fea5f938..e69ab670fb84e 100644 --- a/include/posix/sys/ioctl.h +++ b/include/posix/sys/ioctl.h @@ -8,7 +8,7 @@ #include -__syscall int sys_ioctl(int fd, unsigned long request, va_list args); +__syscall int sys_ioctl(int fd, unsigned long request, long n_args, uintptr_t *args); int ioctl(int fd, unsigned long request, ...); #define FIONBIO 0x5421 diff --git a/include/sys/fdtable.h b/include/sys/fdtable.h index 3f1aada23858d..4485609590de7 100644 --- a/include/sys/fdtable.h +++ b/include/sys/fdtable.h @@ -22,7 +22,8 @@ extern "C" { struct fd_op_vtable { ssize_t (*read)(void *obj, void *buf, size_t sz); ssize_t (*write)(void *obj, const void *buf, size_t sz); - int (*ioctl)(void *obj, unsigned int request, va_list args); + int (*ioctl)(void *obj, unsigned long request, + long n_args, uintptr_t *args); }; /** @@ -113,14 +114,20 @@ void *z_get_fd_obj_and_vtable(int fd, const struct fd_op_vtable **vtable); * @param ... Variadic arguments to ioctl */ static inline int z_fdtable_call_ioctl(const struct fd_op_vtable *vtable, void *obj, - unsigned long request, ...) + unsigned long request, int n_args, ...) { - va_list args; - int res; - - va_start(args, request); - res = vtable->ioctl(obj, request, args); - va_end(args); + va_list varargs; + int i, res; + uintptr_t args[3]; + + __ASSERT_NO_MSG(n_args <= ARRAY_SIZE(args)); + + va_start(varargs, n_args); + for (i = 0; i < n_args; i++) { + args[i] = va_arg(varargs, uintptr_t); + } + res = vtable->ioctl(obj, request, n_args, args); + va_end(varargs); return res; } diff --git a/lib/os/fdtable.c b/lib/os/fdtable.c index e3055adfafd68..e08e75cbeeb14 100644 --- a/lib/os/fdtable.c +++ b/lib/os/fdtable.c @@ -24,6 +24,15 @@ #endif #include +/* Number of arguments which can be passed to public ioctl calls + * (i.e. from userspace to kernel space). + * Arbitrary value is supported (at the expense of stack usage). Can + * be increased when ioctl's with more arguments are added. + * Note that kernelspace-kernelspace ioctl calls are handled + * differently (in z_fdtable_call_ioctl()). + */ +#define MAX_USERSPACE_IOCTL_ARGS 1 + struct fd_entry { void *obj; const struct fd_op_vtable *vtable; @@ -238,7 +247,7 @@ int z_impl_sys_close(int fd) return -1; } - res = z_fdtable_call_ioctl(fdtable[fd].vtable, fdtable[fd].obj, ZFD_IOCTL_CLOSE); + res = z_fdtable_call_ioctl(fdtable[fd].vtable, fdtable[fd].obj, ZFD_IOCTL_CLOSE, 0); z_free_fd(fd); return res; @@ -264,7 +273,8 @@ int fsync(int fd) return -1; } - return z_fdtable_call_ioctl(fdtable[fd].vtable, fdtable[fd].obj, ZFD_IOCTL_FSYNC); + return z_fdtable_call_ioctl(fdtable[fd].vtable, fdtable[fd].obj, + ZFD_IOCTL_FSYNC, 0); } off_t lseek(int fd, off_t offset, int whence) @@ -273,51 +283,78 @@ off_t lseek(int fd, off_t offset, int whence) return -1; } - return z_fdtable_call_ioctl(fdtable[fd].vtable, fdtable[fd].obj, ZFD_IOCTL_LSEEK, - offset, whence); + return z_fdtable_call_ioctl(fdtable[fd].vtable, fdtable[fd].obj, + ZFD_IOCTL_LSEEK, + 2, offset, whence); } FUNC_ALIAS(lseek, _lseek, off_t); -int z_impl_sys_ioctl(int fd, unsigned long request, va_list args) +int z_impl_sys_ioctl(int fd, unsigned long request, long n_args, uintptr_t *args) { if (_check_fd(fd) < 0) { return -1; } - return fdtable[fd].vtable->ioctl(fdtable[fd].obj, request, args); + return fdtable[fd].vtable->ioctl(fdtable[fd].obj, request, n_args, args); } #ifdef CONFIG_USERSPACE -ssize_t z_vrfy_sys_ioctl(int fd, unsigned long request, va_list args) +ssize_t z_vrfy_sys_ioctl(int fd, unsigned long request, long n_args, uintptr_t *args) { -/* This doesn't work so far, until we start to fish inside a particular - * va_list structure for a particular arch. Maybe later. - */ -#if 0 - /* Check that we can access 4 words of arguments. This is very - * generic check, implementation of specific ioctl's are - * expected to perform their own detailed argument checking. - */ - Z_OOPS(Z_SYSCALL_MEMORY_READ(args, sizeof(uintptr_t) * 4)); -#endif + Z_OOPS(Z_SYSCALL_MEMORY_READ(args, sizeof(*args) * n_args)); if (request >= ZFD_IOCTL_PRIVATE) { errno = EINVAL; return -1; } - return z_impl_sys_ioctl(fd, request, args); + return z_impl_sys_ioctl(fd, request, n_args, args); } #include #endif /* CONFIG_USERSPACE */ +static int _vioctl(int fd, unsigned long request, va_list args) +{ + int i, n_args; + /* We assume that for argument passing [on stack], natural word size + * of the plaform is used. So for example, for LP64 platform, where + * int is 32-bit, it's still pushed as 64-bit value on stack. + */ + uintptr_t marshalled_args[MAX_USERSPACE_IOCTL_ARGS]; + + /* Calculate number of arguments for individual ioctl requests. */ + switch (request) { + case F_GETFL: + n_args = 0; + break; + case F_SETFL: + n_args = 1; + break; + default: + errno = EINVAL; + return -1; + } + + if (n_args > ARRAY_SIZE(marshalled_args)) { + /* Use distinguishable error code. */ + errno = EDOM; + return -1; + } + + for (i = 0; i < n_args; i++) { + marshalled_args[i] = va_arg(args, uintptr_t); + } + + return sys_ioctl(fd, request, n_args, marshalled_args); +} + int ioctl(int fd, unsigned long request, ...) { va_list args; int res; va_start(args, request); - res = sys_ioctl(fd, request, args); + res = _vioctl(fd, request, args); va_end(args); return res; @@ -344,7 +381,7 @@ int fcntl(int fd, int cmd, ...) /* The rest of commands are per-fd, handled by ioctl vmethod. */ va_start(args, cmd); - res = sys_ioctl(fd, cmd, args); + res = _vioctl(fd, cmd, args); va_end(args); return res; @@ -373,7 +410,8 @@ static ssize_t stdinout_write_vmeth(void *obj, const void *buffer, size_t count) #endif } -static int stdinout_ioctl_vmeth(void *obj, unsigned int request, va_list args) +static int stdinout_ioctl_vmeth(void *obj, unsigned long request, + long n_args, uintptr_t *args) { errno = EINVAL; return -1; diff --git a/lib/posix/eventfd.c b/lib/posix/eventfd.c index 27d9c9487d577..1ddd26b57ac92 100644 --- a/lib/posix/eventfd.c +++ b/lib/posix/eventfd.c @@ -133,7 +133,8 @@ static ssize_t eventfd_write_op(void *obj, const void *buf, size_t sz) return sizeof(eventfd_t); } -static int eventfd_ioctl_op(void *obj, unsigned int request, va_list args) +static int eventfd_ioctl_op(void *obj, unsigned long request, + long n_args, uintptr_t *args) { struct eventfd *efd = (struct eventfd *)obj; @@ -144,7 +145,7 @@ static int eventfd_ioctl_op(void *obj, unsigned int request, va_list args) case F_SETFL: { int flags; - flags = va_arg(args, int); + flags = (int)args[0]; if (flags & ~EFD_FLAGS_SET) { errno = EINVAL; @@ -165,9 +166,9 @@ static int eventfd_ioctl_op(void *obj, unsigned int request, va_list args) struct k_poll_event **pev; struct k_poll_event *pev_end; - pfd = va_arg(args, struct zsock_pollfd *); - pev = va_arg(args, struct k_poll_event **); - pev_end = va_arg(args, struct k_poll_event *); + pfd = (struct zsock_pollfd *)args[0]; + pev = (struct k_poll_event **)args[1]; + pev_end = (struct k_poll_event *)args[2]; return eventfd_poll_prepare(obj, pfd, pev, pev_end); } @@ -176,8 +177,8 @@ static int eventfd_ioctl_op(void *obj, unsigned int request, va_list args) struct zsock_pollfd *pfd; struct k_poll_event **pev; - pfd = va_arg(args, struct zsock_pollfd *); - pev = va_arg(args, struct k_poll_event **); + pfd = (struct zsock_pollfd *)args[0]; + pev = (struct k_poll_event **)args[1]; return eventfd_poll_update(obj, pfd, pev); } diff --git a/lib/posix/fs.c b/lib/posix/fs.c index ba1ed9a9f7814..e81be5a8bf503 100644 --- a/lib/posix/fs.c +++ b/lib/posix/fs.c @@ -93,7 +93,8 @@ int open(const char *name, int flags) return fd; } -static int fs_ioctl_vmeth(void *obj, unsigned int request, va_list args) +static int fs_ioctl_vmeth(void *obj, unsigned long request, + long n_args, uintptr_t *args) { int rc; struct posix_fs_desc *ptr = obj; @@ -108,8 +109,8 @@ static int fs_ioctl_vmeth(void *obj, unsigned int request, va_list args) off_t offset; int whence; - offset = va_arg(args, off_t); - whence = va_arg(args, int); + offset = (off_t)args[0]; + whence = (int)args[1]; rc = fs_seek(&ptr->file, offset, whence); break; diff --git a/subsys/net/lib/sockets/socketpair.c b/subsys/net/lib/sockets/socketpair.c index fb5be1e69be53..7e1abea42a7c2 100644 --- a/subsys/net/lib/sockets/socketpair.c +++ b/subsys/net/lib/sockets/socketpair.c @@ -879,7 +879,8 @@ static int zsock_poll_update_ctx(struct spair *const spair, return res; } -static int spair_ioctl(void *obj, unsigned int request, va_list args) +static int spair_ioctl(void *obj, unsigned long request, + long n_args, uintptr_t *args) { int res; struct zsock_pollfd *pfd; @@ -915,7 +916,7 @@ static int spair_ioctl(void *obj, unsigned int request, va_list args) } case F_SETFL: { - flags = va_arg(args, int); + flags = (int)args[0]; if (flags & O_NONBLOCK) { spair->flags |= SPAIR_FLAG_NONBLOCK; @@ -936,17 +937,17 @@ static int spair_ioctl(void *obj, unsigned int request, va_list args) } case ZFD_IOCTL_POLL_PREPARE: { - pfd = va_arg(args, struct zsock_pollfd *); - pev = va_arg(args, struct k_poll_event **); - pev_end = va_arg(args, struct k_poll_event *); + pfd = (struct zsock_pollfd *)args[0]; + pev = (struct k_poll_event **)args[1]; + pev_end = (struct k_poll_event *)args[2]; res = zsock_poll_prepare_ctx(obj, pfd, pev, pev_end); goto out; } case ZFD_IOCTL_POLL_UPDATE: { - pfd = va_arg(args, struct zsock_pollfd *); - pev = va_arg(args, struct k_poll_event **); + pfd = (struct zsock_pollfd *)args[0]; + pev = (struct k_poll_event **)args[1]; res = zsock_poll_update_ctx(obj, pfd, pev); goto out; diff --git a/subsys/net/lib/sockets/sockets_can.c b/subsys/net/lib/sockets/sockets_can.c index e549bd73eb45a..58778ee6f41d9 100644 --- a/subsys/net/lib/sockets/sockets_can.c +++ b/subsys/net/lib/sockets/sockets_can.c @@ -407,7 +407,8 @@ static int can_close_socket(struct net_context *ctx) return 0; } -static int can_sock_ioctl_vmeth(void *obj, unsigned int request, va_list args) +static int can_sock_ioctl_vmeth(void *obj, unsigned long request, + long n_args, uintptr_t *args) { if (request == ZFD_IOCTL_CLOSE) { int ret; @@ -418,7 +419,7 @@ static int can_sock_ioctl_vmeth(void *obj, unsigned int request, va_list args) } } - return sock_fd_op_vtable.fd_vtable.ioctl(obj, request, args); + return sock_fd_op_vtable.fd_vtable.ioctl(obj, request, n_args, args); } /* diff --git a/subsys/net/lib/sockets/sockets_net_mgmt.c b/subsys/net/lib/sockets/sockets_net_mgmt.c index 7f0fa646876f1..8f6c9ba8ab05c 100644 --- a/subsys/net/lib/sockets/sockets_net_mgmt.c +++ b/subsys/net/lib/sockets/sockets_net_mgmt.c @@ -302,8 +302,8 @@ static ssize_t net_mgmt_sock_write(void *obj, const void *buffer, return znet_mgmt_sendto(obj, buffer, count, 0, NULL, 0); } -static int net_mgmt_sock_ioctl(void *obj, unsigned int request, - va_list args) +static int net_mgmt_sock_ioctl(void *obj, unsigned long request, + long n_args, uintptr_t *args) { return 0; } diff --git a/subsys/net/lib/sockets/sockets_packet.c b/subsys/net/lib/sockets/sockets_packet.c index 41fafd40d6a8c..d91cf7d6050fa 100644 --- a/subsys/net/lib/sockets/sockets_packet.c +++ b/subsys/net/lib/sockets/sockets_packet.c @@ -280,10 +280,10 @@ static ssize_t packet_sock_write_vmeth(void *obj, const void *buffer, return zpacket_sendto_ctx(obj, buffer, count, 0, NULL, 0); } -static int packet_sock_ioctl_vmeth(void *obj, unsigned int request, - va_list args) +static int packet_sock_ioctl_vmeth(void *obj, unsigned long request, + long n_args, uintptr_t *args) { - return sock_fd_op_vtable.fd_vtable.ioctl(obj, request, args); + return sock_fd_op_vtable.fd_vtable.ioctl(obj, request, n_args, args); } /* diff --git a/subsys/net/lib/sockets/sockets_tls.c b/subsys/net/lib/sockets/sockets_tls.c index a477317bd9c08..a6c9a28169e08 100644 --- a/subsys/net/lib/sockets/sockets_tls.c +++ b/subsys/net/lib/sockets/sockets_tls.c @@ -1936,7 +1936,8 @@ static ssize_t tls_sock_write_vmeth(void *obj, const void *buffer, return ztls_sendto_ctx(obj, buffer, count, 0, NULL, 0); } -static int tls_sock_ioctl_vmeth(void *obj, unsigned int request, va_list args) +static int tls_sock_ioctl_vmeth(void *obj, unsigned long request, + long n_args, uintptr_t *args) { switch (request) { @@ -1945,7 +1946,8 @@ static int tls_sock_ioctl_vmeth(void *obj, unsigned int request, va_list args) case F_SETFL: case ZFD_IOCTL_GETSOCKNAME: /* Pass the call to the core socket implementation. */ - return sock_fd_op_vtable.fd_vtable.ioctl(obj, request, args); + return sock_fd_op_vtable.fd_vtable.ioctl(obj, request, + n_args, args); case ZFD_IOCTL_CLOSE: return ztls_close_ctx(obj); @@ -1955,9 +1957,9 @@ static int tls_sock_ioctl_vmeth(void *obj, unsigned int request, va_list args) struct k_poll_event **pev; struct k_poll_event *pev_end; - pfd = va_arg(args, struct zsock_pollfd *); - pev = va_arg(args, struct k_poll_event **); - pev_end = va_arg(args, struct k_poll_event *); + pfd = (struct zsock_pollfd *)args[0]; + pev = (struct k_poll_event **)args[1]; + pev_end = (struct k_poll_event *)args[2]; return ztls_poll_prepare_ctx(obj, pfd, pev, pev_end); } @@ -1966,8 +1968,8 @@ static int tls_sock_ioctl_vmeth(void *obj, unsigned int request, va_list args) struct zsock_pollfd *pfd; struct k_poll_event **pev; - pfd = va_arg(args, struct zsock_pollfd *); - pev = va_arg(args, struct k_poll_event **); + pfd = (struct zsock_pollfd *)args[0]; + pev = (struct k_poll_event **)args[1]; return ztls_poll_update_ctx(obj, pfd, pev); } diff --git a/subsys/net/lib/websocket/websocket.c b/subsys/net/lib/websocket/websocket.c index 7019a02cacb8d..0c12dd3c8e8da 100644 --- a/subsys/net/lib/websocket/websocket.c +++ b/subsys/net/lib/websocket/websocket.c @@ -417,7 +417,8 @@ int websocket_disconnect(int ws_sock) return ret; } -static int websocket_ioctl_vmeth(void *obj, unsigned int request, va_list args) +static int websocket_ioctl_vmeth(void *obj, unsigned long request, + long n_args, uintptr_t *args) { if (request == ZFD_IOCTL_CLOSE) { struct websocket_context *ctx = obj; @@ -434,7 +435,7 @@ static int websocket_ioctl_vmeth(void *obj, unsigned int request, va_list args) return ret; } - return sock_fd_op_vtable.fd_vtable.ioctl(obj, request, args); + return sock_fd_op_vtable.fd_vtable.ioctl(obj, request, n_args, args); } static int websocket_prepare_and_send(struct websocket_context *ctx, From 58a19b89af9943a9048d065cdb54500c3df089fa Mon Sep 17 00:00:00 2001 From: Paul Sokolovsky Date: Fri, 22 May 2020 16:16:58 +0300 Subject: [PATCH 5/5] net: sockets: Pass explicit # of arguments to z_fdtable_call_ioctl() Following a refactor to allow pass ioctl arguments across userspace/kernel boundary. Signed-off-by: Paul Sokolovsky --- subsys/net/lib/sockets/sockets.c | 30 ++++++++++++++-------------- subsys/net/lib/sockets/sockets_tls.c | 4 ++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/subsys/net/lib/sockets/sockets.c b/subsys/net/lib/sockets/sockets.c index 0baaf11d21ebf..0453bea2f27db 100644 --- a/subsys/net/lib/sockets/sockets.c +++ b/subsys/net/lib/sockets/sockets.c @@ -258,7 +258,7 @@ int z_impl_zsock_close(int sock) NET_DBG("close: ctx=%p, fd=%d", ctx, sock); return z_fdtable_call_ioctl((const struct fd_op_vtable *)vtable, - ctx, ZFD_IOCTL_CLOSE); + ctx, ZFD_IOCTL_CLOSE, 0); } #ifdef CONFIG_USERSPACE @@ -1095,7 +1095,7 @@ int z_impl_zsock_fcntl(int sock, int cmd, int flags) } return z_fdtable_call_ioctl((const struct fd_op_vtable *)vtable, - obj, cmd, flags); + obj, cmd, 1, flags); } #ifdef CONFIG_USERSPACE @@ -1206,7 +1206,7 @@ int z_impl_zsock_poll(struct zsock_pollfd *fds, int nfds, int poll_timeout) result = z_fdtable_call_ioctl(vtable, ctx, ZFD_IOCTL_POLL_PREPARE, - pfd, &pev, pev_end); + 3, pfd, &pev, pev_end); if (result == -EALREADY) { /* If POLL_PREPARE returned with EALREADY, it means * it already detected that some socket is ready. In @@ -1224,7 +1224,7 @@ int z_impl_zsock_poll(struct zsock_pollfd *fds, int nfds, int poll_timeout) */ return z_fdtable_call_ioctl(vtable, ctx, ZFD_IOCTL_POLL_OFFLOAD, - fds, nfds, poll_timeout); + 3, fds, nfds, poll_timeout); } else if (result != 0) { errno = -result; return -1; @@ -1274,7 +1274,7 @@ int z_impl_zsock_poll(struct zsock_pollfd *fds, int nfds, int poll_timeout) result = z_fdtable_call_ioctl(vtable, ctx, ZFD_IOCTL_POLL_UPDATE, - pfd, &pev); + 2, pfd, &pev); if (result == -EAGAIN) { retry = true; continue; @@ -1633,7 +1633,7 @@ int z_impl_zsock_getsockname(int sock, struct sockaddr *addr, NET_DBG("getsockname: ctx=%p, fd=%d", ctx, sock); return z_fdtable_call_ioctl((const struct fd_op_vtable *)vtable, ctx, - ZFD_IOCTL_GETSOCKNAME, addr, addrlen); + ZFD_IOCTL_GETSOCKNAME, 2, addr, addrlen); } #ifdef CONFIG_USERSPACE @@ -1676,7 +1676,7 @@ static ssize_t sock_write_vmeth(void *obj, const void *buffer, size_t count) return zsock_sendto_ctx(obj, buffer, count, 0, NULL, 0); } -static int sock_ioctl_vmeth(void *obj, unsigned int request, va_list args) +static int sock_ioctl_vmeth(void *obj, unsigned long request, long n_args, uintptr_t *args) { switch (request) { @@ -1691,7 +1691,7 @@ static int sock_ioctl_vmeth(void *obj, unsigned int request, va_list args) case F_SETFL: { int flags; - flags = va_arg(args, int); + flags = (int)args[0]; if (flags & O_NONBLOCK) { sock_set_flag(obj, SOCK_NONBLOCK, SOCK_NONBLOCK); @@ -1710,9 +1710,9 @@ static int sock_ioctl_vmeth(void *obj, unsigned int request, va_list args) struct k_poll_event **pev; struct k_poll_event *pev_end; - pfd = va_arg(args, struct zsock_pollfd *); - pev = va_arg(args, struct k_poll_event **); - pev_end = va_arg(args, struct k_poll_event *); + pfd = (struct zsock_pollfd *)args[0]; + pev = (struct k_poll_event **)args[1]; + pev_end = (struct k_poll_event *)args[2]; return zsock_poll_prepare_ctx(obj, pfd, pev, pev_end); } @@ -1721,8 +1721,8 @@ static int sock_ioctl_vmeth(void *obj, unsigned int request, va_list args) struct zsock_pollfd *pfd; struct k_poll_event **pev; - pfd = va_arg(args, struct zsock_pollfd *); - pev = va_arg(args, struct k_poll_event **); + pfd = (struct zsock_pollfd *)args[0]; + pev = (struct k_poll_event **)args[1]; return zsock_poll_update_ctx(obj, pfd, pev); } @@ -1731,8 +1731,8 @@ static int sock_ioctl_vmeth(void *obj, unsigned int request, va_list args) struct sockaddr *addr; socklen_t *addrlen; - addr = va_arg(args, struct sockaddr *); - addrlen = va_arg(args, socklen_t *); + addr = (struct sockaddr *)args[0]; + addrlen = (socklen_t *)args[1]; return zsock_getsockname_ctx(obj, addr, addrlen); } diff --git a/subsys/net/lib/sockets/sockets_tls.c b/subsys/net/lib/sockets/sockets_tls.c index a6c9a28169e08..ec0a3640a54a3 100644 --- a/subsys/net/lib/sockets/sockets_tls.c +++ b/subsys/net/lib/sockets/sockets_tls.c @@ -1197,7 +1197,7 @@ int ztls_close_ctx(struct net_context *ctx) err = -EBADF; } - ret = z_fdtable_call_ioctl(&sock_fd_op_vtable.fd_vtable, ctx, ZFD_IOCTL_CLOSE); + ret = z_fdtable_call_ioctl(&sock_fd_op_vtable.fd_vtable, ctx, ZFD_IOCTL_CLOSE, 0); /* In case close fails, we propagate errno value set by close. * In case close succeeds, but tls_release fails, set errno @@ -1331,7 +1331,7 @@ int ztls_accept_ctx(struct net_context *parent, struct sockaddr *addr, __ASSERT(err == 0, "TLS context release failed"); } - err = z_fdtable_call_ioctl(&sock_fd_op_vtable.fd_vtable, child, ZFD_IOCTL_CLOSE); + err = z_fdtable_call_ioctl(&sock_fd_op_vtable.fd_vtable, child, ZFD_IOCTL_CLOSE, 0); __ASSERT(err == 0, "Child socket close failed"); z_free_fd(fd);