From 6918593dfa74b08d5a50a0ee8fe2677f4334d7ee Mon Sep 17 00:00:00 2001 From: julianuziemblo Date: Fri, 23 May 2025 20:22:33 +0200 Subject: [PATCH 1/3] ioctl: serialize special ioctls outside of kernel For the kernel, the ioctl syscall data should be transparent, so special ioctls (e.g. ones containing a pointer in the input structure) should be serialized/deserialized in userspace. JIRA: RTOS-1205 --- include/ioctl.h | 1 + posix/posix.c | 80 ++++++++++++++++++------------------------- posix/posix_private.h | 20 ----------- syscalls.c | 1 + 4 files changed, 35 insertions(+), 67 deletions(-) diff --git a/include/ioctl.h b/include/ioctl.h index 255ae2297..cc8533608 100644 --- a/include/ioctl.h +++ b/include/ioctl.h @@ -19,6 +19,7 @@ typedef struct { unsigned long request; + unsigned long size; char data[]; } ioctl_in_t; diff --git a/posix/posix.c b/posix/posix.c index cb6a14d2c..eb4ab903e 100644 --- a/posix/posix.c +++ b/posix/posix.c @@ -1644,22 +1644,15 @@ int posix_fcntl(int fd, unsigned int cmd, u8 *ustack) #define IOCPARM_MASK 0x1fffUL #define IOCPARM_LEN(x) (((x) >> 16) & IOCPARM_MASK) -#define IOC_OUT 0x40000000UL -#define IOC_IN 0x80000000UL -#define IOC_INOUT (IOC_IN | IOC_OUT) -#define _IOC(inout, group, num, len) ((unsigned long)((inout) | (((len) & IOCPARM_MASK) << 16) | (((unsigned int)(group)) << 8) | (num))) +#define IOC_VOID 0x20000000UL +#define IOC_OUT 0x40000000UL +#define IOC_IN 0x80000000UL +#define IOC_INOUT (IOC_IN | IOC_OUT) -#define SIOCGIFCONF _IOC(IOC_INOUT, 'S', 0x12U, sizeof(struct ifconf)) -#define SIOCADDRT _IOC(IOC_IN, 'S', 0x44U, sizeof(struct rtentry)) -#define SIOCDELRT _IOC(IOC_IN, 'S', 0x45U, sizeof(struct rtentry)) - -static void ioctl_pack(msg_t *msg, unsigned long request, void *data, oid_t *oid) +static void ioctl_pack(msg_t *msg, unsigned long request, void *data, size_t size, oid_t *oid) { - size_t size = IOCPARM_LEN(request); ioctl_in_t *ioctl = (ioctl_in_t *)msg->i.raw; - struct ifconf *ifc; - struct rtentry *rt; hal_memcpy(&msg->oid, oid, sizeof(*oid)); msg->type = mtDevCtl; @@ -1669,6 +1662,7 @@ static void ioctl_pack(msg_t *msg, unsigned long request, void *data, oid_t *oid msg->o.size = 0; ioctl->request = request; + ioctl->size = size; if ((request & IOC_INOUT) != 0U) { if ((request & IOC_IN) != 0U) { @@ -1692,34 +1686,14 @@ static void ioctl_pack(msg_t *msg, unsigned long request, void *data, oid_t *oid hal_memcpy(ioctl->data, &data, size); } else { - /* No action required */ - } - - - /* ioctl special case: arg is structure with pointer - has to be custom-packed into message */ - if (request == SIOCGIFCONF) { - ifc = (struct ifconf *)data; - msg->o.data = ifc->ifc_buf; - msg->o.size = ifc->ifc_len; - } - else if ((request == SIOCADDRT) || (request == SIOCDELRT)) { - rt = (struct rtentry *)data; - if (rt->rt_dev != NULL) { - msg->o.data = rt->rt_dev; - msg->o.size = hal_strlen(rt->rt_dev) + 1U; - } - } - else { - /* No action required */ + /* Nothing to do */ } } -static int ioctl_processResponse(const msg_t *msg, unsigned long request, void *data) +static int ioctl_processResponse(const msg_t *msg, unsigned long request, void *data, size_t size) { - size_t size = IOCPARM_LEN(request); int err; - struct ifconf *ifc; err = msg->o.err; @@ -1727,11 +1701,6 @@ static int ioctl_processResponse(const msg_t *msg, unsigned long request, void * hal_memcpy(data, msg->o.raw, size); } - if (request == SIOCGIFCONF) { /* restore overridden userspace pointer */ - ifc = (struct ifconf *)data; - ifc->ifc_buf = msg->o.data; - } - return err; } @@ -1744,19 +1713,36 @@ int posix_ioctl(int fildes, unsigned long request, u8 *ustack) int err; msg_t msg; void *data = NULL; + size_t size = IOCPARM_LEN(request); err = posix_getOpenFile(fildes, &f); - if (err == 0) { - /* TODO: handle POSIX defined requests with `switch (request)` */ - if (((request & IOC_INOUT) != 0U) || (IOCPARM_LEN(request) > 0U)) { - GETFROMSTACK(ustack, void *, data, 2U); + if (err == EOK) { + /* TODO: handle POSIX defined requests */ + if (size > 0U) { + GETFROMSTACK(ustack, void *, data, 2); + /* the actual size of the pointed-to structure: >= IOCPARM_LEN(request) */ + GETFROMSTACK(ustack, size_t, size, 3); + + if ((request & IOC_INOUT) != 0U) { + if (data == NULL) { + err = -EFAULT; + } + else if (vm_mapBelongs(proc_current()->process, data, size) < 0) { + err = -EFAULT; + } + else { + /* Nothing to do */ + } + } } - ioctl_pack(&msg, request, data, &f->oid); - - err = proc_send(f->oid.port, &msg); if (err == EOK) { - err = ioctl_processResponse(&msg, request, data); + ioctl_pack(&msg, request, data, size, &f->oid); + + err = proc_send(f->oid.port, &msg); + if (err == EOK) { + err = ioctl_processResponse(&msg, request, data, size); + } } (void)posix_fileDeref(f); diff --git a/posix/posix_private.h b/posix/posix_private.h index 5ad9e1a6f..79b873e89 100644 --- a/posix/posix_private.h +++ b/posix/posix_private.h @@ -123,26 +123,6 @@ typedef struct _process_info_t { } process_info_t; -/* SIOCGIFCONF ioctl special case: arg is structure with pointer */ -struct ifconf { - unsigned int ifc_len; /* size of buffer */ - char *ifc_buf; /* buffer address */ -}; - -/* SIOADDRT and SIOCDELRT ioctls special case: arg is structure with pointer */ -struct rtentry { - struct sockaddr rt_dst; - struct sockaddr rt_gateway; - struct sockaddr rt_genmask; - short rt_flags; - short rt_metric; - char *rt_dev; - unsigned long rt_mss; - unsigned long rt_window; - unsigned short rt_irtt; -}; - - int posix_fileDeref(open_file_t *f); diff --git a/syscalls.c b/syscalls.c index 1125eb037..f609af5cc 100644 --- a/syscalls.c +++ b/syscalls.c @@ -1762,6 +1762,7 @@ int syscalls_sys_ioctl(u8 *ustack) GETFROMSTACK(ustack, int, fildes, 0U); GETFROMSTACK(ustack, unsigned long, request, 1U); + /* vm_mapBelongs on optional data pointer checked in posix_ioctl */ return posix_ioctl(fildes, request, ustack); } From 4f5cc9295418e9c4b87bfac18571db680ca2ef40 Mon Sep 17 00:00:00 2001 From: julianuziemblo Date: Thu, 22 Jan 2026 13:30:46 +0100 Subject: [PATCH 2/3] !ioctl: change ioctl request number interface First, there's no need for ioctl direction to span across 3 bits, when 2 is enough to represent all possible states, including IOC_VOID. The only thing to be weary of is now request cannot be checked with `request & IOC_VOID` as that will always produce a value of 0. However, that was not idiomatic anyways - the idiomatic way is to check `(request & IOC_INOUT) == 0`. Second, make 29th bit of ioctl request number IOC_NESTED. If this bit is set, it means the structure passed in the ioctl command is a nested one and has to be serialized (flattened) in the userspace before passing through kernel. JIRA: RTOS-1205 --- posix/posix.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/posix/posix.c b/posix/posix.c index eb4ab903e..549cf111f 100644 --- a/posix/posix.c +++ b/posix/posix.c @@ -1644,10 +1644,11 @@ int posix_fcntl(int fd, unsigned int cmd, u8 *ustack) #define IOCPARM_MASK 0x1fffUL #define IOCPARM_LEN(x) (((x) >> 16) & IOCPARM_MASK) -#define IOC_VOID 0x20000000UL -#define IOC_OUT 0x40000000UL -#define IOC_IN 0x80000000UL -#define IOC_INOUT (IOC_IN | IOC_OUT) +#define IOC_VOID 0x00000000UL +#define IOC_NESTED 0x20000000UL +#define IOC_OUT 0x40000000UL +#define IOC_IN 0x80000000UL +#define IOC_INOUT (IOC_IN | IOC_OUT) static void ioctl_pack(msg_t *msg, unsigned long request, void *data, size_t size, oid_t *oid) From c028af9b98e65297e9db46a8b1b08379c46565e6 Mon Sep 17 00:00:00 2001 From: julianuziemblo Date: Wed, 18 Feb 2026 11:12:28 +0100 Subject: [PATCH 3/3] posix: transfer common ioctl defines to include/ioctl Common defines in kernel to keep the ABI consistent JIRA: RTOS-1205 --- include/ioctl.h | 13 +++++++++++++ posix/posix.c | 10 ---------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/include/ioctl.h b/include/ioctl.h index cc8533608..2633418fc 100644 --- a/include/ioctl.h +++ b/include/ioctl.h @@ -16,6 +16,19 @@ #ifndef _PH_IOCTL_H_ #define _PH_IOCTL_H_ +#define IOCPARM_MASK 0x1fffUL +#define IOCPARM_LEN(x) (((x) >> 16) & IOCPARM_MASK) +#define IOCBASECMD(x) ((x) & ~(IOCPARM_MASK << 16)) +#define IOCGROUP(x) (((x) >> 8) & 0xffU) + +#define IOC_VOID 0x00000000UL +#define IOC_NESTED 0x20000000UL +#define IOC_OUT 0x40000000UL +#define IOC_IN 0x80000000UL +#define IOC_INOUT (IOC_IN | IOC_OUT) +#define IOC_DIRMASK 0xc0000000UL + +#define _IOC(inout, group, num, len) ((unsigned long)((inout) | (((len) & IOCPARM_MASK) << 16) | ((group) << 8) | (num))) typedef struct { unsigned long request; diff --git a/posix/posix.c b/posix/posix.c index 549cf111f..53537997d 100644 --- a/posix/posix.c +++ b/posix/posix.c @@ -1641,16 +1641,6 @@ int posix_fcntl(int fd, unsigned int cmd, u8 *ustack) } -#define IOCPARM_MASK 0x1fffUL -#define IOCPARM_LEN(x) (((x) >> 16) & IOCPARM_MASK) - -#define IOC_VOID 0x00000000UL -#define IOC_NESTED 0x20000000UL -#define IOC_OUT 0x40000000UL -#define IOC_IN 0x80000000UL -#define IOC_INOUT (IOC_IN | IOC_OUT) - - static void ioctl_pack(msg_t *msg, unsigned long request, void *data, size_t size, oid_t *oid) { ioctl_in_t *ioctl = (ioctl_in_t *)msg->i.raw;