From 52b6047f45461954be9661753fbd60a31e1fcd86 Mon Sep 17 00:00:00 2001 From: Jonas Jelonek Date: Fri, 11 Oct 2024 10:36:26 +0200 Subject: [PATCH 1/2] fs: ioctl: export constants for direction values Exports IOC_DIR_* constants to use for the direction parameter instead of plain integer values. The constants are based on the target's _IOC_* definitions. Signed-off-by: Jonas Jelonek --- lib/fs.c | 88 ++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 53 insertions(+), 35 deletions(-) diff --git a/lib/fs.c b/lib/fs.c index e3f8a4ab..b6afb18c 100644 --- a/lib/fs.c +++ b/lib/fs.c @@ -62,7 +62,17 @@ #include #if defined(__linux__) +#define HAS_IOCTL +#endif + +#ifdef HAS_IOCTL #include + +#define IOC_DIR_NONE (_IOC_NONE) +#define IOC_DIR_READ (_IOC_READ) +#define IOC_DIR_WRITE (_IOC_WRITE) +#define IOC_DIR_RW (_IOC_READ | _IOC_WRITE) + #endif #include "ucode/module.h" @@ -937,50 +947,50 @@ uc_fs_fileno(uc_vm_t *vm, size_t nargs) return uc_fs_fileno_common(vm, nargs, "fs.file"); } -#if defined(__linux__) +#ifdef HAS_IOCTL /** * Performs an ioctl operation on the file. - * + * * The direction parameter specifies who is reading and writing, * from the user's point of view. It can be one of the following values: - * - * | Direction | Description | - * |-----------|-----------------------------------------------------------------------------------------| - * | 0 | NONE - neither userspace nor kernel is writing, ioctl is executed without passing data. | - * | 1 | WRITE - userspace is writing and kernel is reading. | - * | 2 | READ - kernel is writing and userspace is reading. | - * | 3 | READ+WRITE - userspace is writing and kernel is writing back into the data structure. | - * + * + * | Direction | Description | + * |----------------|-----------------------------------------------------------------------------------| + * | IOC_DIR_NONE | neither userspace nor kernel is writing, ioctl is executed without passing data. | + * | IOC_DIR_WRITE | userspace is writing and kernel is reading. | + * | IOC_DIR_READ | kernel is writing and userspace is reading. | + * | IOC_DIR_RW | userspace is writing and kernel is writing back into the data structure. | + * * The size parameter has a different purpose depending on the direction parameter: - * - direction = 0 -> the size parameter is not used - * - direction = 1 -> size must be the length (in bytes) of argp - * - direction = 2 -> expected length (in bytes) of the data returned by kernel - * - direction = 3 -> size is the length (in bytes) of argp, and the length of the data returned by kernel. - * - * The argp parameter should be the data to be written for direction '1' and '3', otherwise null. - * - * Returns the result of the ioctl operation; for direction '2' and '3' this is a string containing + * - IOC_DIR_NONE -> the size parameter is not used + * - IOC_DIR_WRITE -> size must be the length (in bytes) of argp + * - IOC_DIR_READ -> expected length (in bytes) of the data returned by kernel + * - IOC_DIR_RW -> size is the length (in bytes) of argp, and the length of the data returned by kernel. + * + * The argp parameter should be the data to be written for IOC_DIR_WRITE and IOC_DIR_RW, otherwise null. + * + * Returns the result of the ioctl operation; for IOC_DIR_READ and IOC_DIR_RW this is a string containing * the data, otherwise a number as return code. * In case of an error, null is returned and the error code is available via last_error. - * + * * @function module:fs.file#ioctl - * + * * @param {number} direction - * The direction of the ioctl operation. - * + * The direction of the ioctl operation. Use constants IOC_DIR_*. + * * @param {number} type * ioctl type (see https://www.kernel.org/doc/html/latest/userspace-api/ioctl/ioctl-number.html) - * + * * @param {number} num * ioctl sequence number. - * + * * @param {number} size * The size of the ioctl operation payload. - * + * * @param {?string} payload * The ioctl payload. - * + * * @returns {?number|?string} */ static uc_value_t * @@ -1016,26 +1026,26 @@ uc_fs_ioctl(uc_vm_t *vm, size_t nargs) nr = ucv_uint64_get(num); switch (dir) { - case 0: /* ioctl w/o read and write */ - req = _IOC(_IOC_NONE, ty, nr, 0); + case IOC_DIR_NONE: + req = _IOC(IOC_DIR_NONE, ty, nr, 0); break; - case 1: /* ioctl write */ + case IOC_DIR_WRITE: if (ucv_type(payload) != UC_STRING) err_return(EINVAL); - req = _IOC(_IOC_WRITE, ty, nr, sz); + req = _IOC(IOC_DIR_WRITE, ty, nr, sz); buf = ucv_string_get(payload); break; - case 2: /* ioctl read */ - req = _IOC(_IOC_READ, ty, nr, sz); + case IOC_DIR_READ: + req = _IOC(IOC_DIR_READ, ty, nr, sz); buf = xalloc(sz); if (!buf) err_return(ENOMEM); freebuf = true; break; - case 3: /* ioctl read+write */ - req = _IOC((_IOC_READ|_IOC_WRITE), ty, nr, sz); + case IOC_DIR_RW: + req = _IOC(IOC_DIR_RW, ty, nr, sz); buf = ucv_string_get(payload); break; default: err_return(EINVAL); @@ -1049,7 +1059,7 @@ uc_fs_ioctl(uc_vm_t *vm, size_t nargs) err_return(errno); } - if (dir >= 2) { + if (dir == IOC_DIR_READ || dir == IOC_DIR_RW) { payload = ucv_string_new_length(buf, sz); if (freebuf) free(buf); @@ -2880,4 +2890,12 @@ void uc_module_init(uc_vm_t *vm, uc_value_t *scope) ucv_object_add(scope, "stdin", uc_resource_new(file_type, stdin)); ucv_object_add(scope, "stdout", uc_resource_new(file_type, stdout)); ucv_object_add(scope, "stderr", uc_resource_new(file_type, stderr)); + +#ifdef HAS_IOCTL +#define ADD_CONST(x) ucv_object_add(scope, #x, ucv_int64_new(x)) + ADD_CONST(IOC_DIR_NONE); + ADD_CONST(IOC_DIR_READ); + ADD_CONST(IOC_DIR_WRITE); + ADD_CONST(IOC_DIR_RW); +#endif } From 033749a3bc6e0aa910fa4e83ed106977e6170f3f Mon Sep 17 00:00:00 2001 From: Jonas Jelonek Date: Fri, 11 Oct 2024 13:21:29 +0200 Subject: [PATCH 2/2] fs: ioctl: improve ioctl read to avoid allocating twice Allocate memory for 'uc_string_t' with the proper size upfront and use it's internal buffer as ioctl buffer. This avoids using 'ucv_string_new_length' and thus allocating memory a second time. For ioctl read-write, this completely reuses the 'uc_string_t', also eliminating the 'ucv_string_new_length' call. Signed-off-by: Jonas Jelonek --- lib/fs.c | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/lib/fs.c b/lib/fs.c index b6afb18c..fb7e2215 100644 --- a/lib/fs.c +++ b/lib/fs.c @@ -1002,12 +1002,12 @@ uc_fs_ioctl(uc_vm_t *vm, size_t nargs) uc_value_t *num = uc_fn_arg(2); uc_value_t *size = uc_fn_arg(3); uc_value_t *payload = uc_fn_arg(4); + uc_string_t *mem = NULL; char *buf = NULL; unsigned long req = 0; unsigned int dir, ty, nr; size_t sz; int fd, ret; - bool freebuf = false; if (!fp) err_return(EBADF); @@ -1027,47 +1027,51 @@ uc_fs_ioctl(uc_vm_t *vm, size_t nargs) switch (dir) { case IOC_DIR_NONE: - req = _IOC(IOC_DIR_NONE, ty, nr, 0); + sz = 0; break; case IOC_DIR_WRITE: if (ucv_type(payload) != UC_STRING) err_return(EINVAL); - req = _IOC(IOC_DIR_WRITE, ty, nr, sz); buf = ucv_string_get(payload); break; case IOC_DIR_READ: - req = _IOC(IOC_DIR_READ, ty, nr, sz); - buf = xalloc(sz); - if (!buf) + mem = xalloc(sizeof(uc_string_t) + sz + 1); + if (!mem) err_return(ENOMEM); - freebuf = true; + mem->header.type = UC_STRING; + mem->header.refcount = 1; + mem->length = sz; + buf = mem->str; + break; case IOC_DIR_RW: - req = _IOC(IOC_DIR_RW, ty, nr, sz); - buf = ucv_string_get(payload); + if (ucv_type(payload) != UC_STRING) + err_return(EINVAL); + + mem = (uc_string_t *)payload; + buf = mem->str; + sz = mem->length; + break; default: err_return(EINVAL); } + req = _IOC(dir, ty, nr, sz); ret = ioctl(fd, req, buf); if (ret < 0) { - if (freebuf) - free(buf); + if (dir == IOC_DIR_READ) + free(mem); err_return(errno); } - if (dir == IOC_DIR_READ || dir == IOC_DIR_RW) { - payload = ucv_string_new_length(buf, sz); - if (freebuf) - free(buf); + if (mem) { + return &mem->header; } else { - payload = ucv_uint64_new(ret); + return ucv_uint64_new(ret); } - - return payload; } #endif