From 42b2af14a28acce2ae3a8640334a40ab78d78bff Mon Sep 17 00:00:00 2001 From: Helge Deller Date: Sat, 28 Dec 2024 19:53:48 +0100 Subject: [PATCH] Fix ioctl_tree_execute() ret type and initialization The umockdev-vala testcase randomly fails because the ioctl() testcase checks the return value pointed to by "ret" in ioctl_tree_execute() which might have stale data as it wasn't initialized. Fix it by always initializing the return code, forcing all code paths to provide a valid return code address, and fixing the VAPI declaration (it's purely an out argument). Co-Authored-By: Martin Pitt https://bugs.debian.org/1091619 --- src/ioctl_tree.c | 6 ++++-- src/ioctl_tree.vapi | 2 +- src/umockdev-ioctl.vala | 8 ++++---- tests/test-ioctl-tree.c | 4 ++-- tests/test-umockdev-vala.vala | 2 +- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/ioctl_tree.c b/src/ioctl_tree.c index d94b6275..b6ab5f37 100644 --- a/src/ioctl_tree.c +++ b/src/ioctl_tree.c @@ -357,6 +357,10 @@ ioctl_tree_execute(ioctl_tree * tree, ioctl_tree * last, IOCTL_REQUEST_TYPE id, ioctl_tree *i; int r, handled; + /* initialize return code */ + assert(ret != NULL); + *ret = -1; + DBG(DBG_IOCTL_TREE, "ioctl_tree_execute ioctl %X\n", (unsigned) id); t = ioctl_type_get_by_id(id); @@ -369,8 +373,6 @@ ioctl_tree_execute(ioctl_tree * tree, ioctl_tree * last, IOCTL_REQUEST_TYPE id, DBG(DBG_IOCTL_TREE, " ioctl_tree_execute: stateless\n"); if (t->execute(NULL, id, arg, &r)) *ret = r; - else - *ret = -1; return last; } diff --git a/src/ioctl_tree.vapi b/src/ioctl_tree.vapi index 68e80936..0a84c248 100644 --- a/src/ioctl_tree.vapi +++ b/src/ioctl_tree.vapi @@ -10,7 +10,7 @@ namespace IoctlTree { [ReturnsModifiedPointer] public void insert(owned Tree node); - public void* execute(void* last, ulong id, void* addr, ref int ret); + public void* execute(void* last, ulong id, void* addr, out int ret); [CCode (instance_pos = -1)] public void write(Posix.FILE f); diff --git a/src/umockdev-ioctl.vala b/src/umockdev-ioctl.vala index 302a619f..45c09a26 100644 --- a/src/umockdev-ioctl.vala +++ b/src/umockdev-ioctl.vala @@ -572,7 +572,7 @@ public class IoctlClient : GLib.Object { IoctlData? data = null; ulong size = IoctlTree.data_size_by_id(_request); ulong type = (_request >> Ioctl._IOC_TYPESHIFT) & ((1 << Ioctl._IOC_TYPEBITS) - 1); - int ret = -1; + int ret; int my_errno; try { @@ -590,7 +590,7 @@ public class IoctlClient : GLib.Object { } else { Posix.errno = Posix.ENOTTY; } - tree.execute(null, _request, *(void**) _arg.data, ref ret); + tree.execute(null, _request, *(void**) _arg.data, out ret); my_errno = Posix.errno; Posix.errno = 0; @@ -897,7 +897,7 @@ internal class IoctlTreeHandler : IoctlBase { ulong request = client.request; ulong size = IoctlTree.data_size_by_id(request); ulong type = (request >> Ioctl._IOC_TYPESHIFT) & ((1 << Ioctl._IOC_TYPEBITS) - 1); - int ret = -1; + int ret; int my_errno; if (tree == null) { @@ -938,7 +938,7 @@ internal class IoctlTreeHandler : IoctlBase { } else { Posix.errno = Posix.ENOTTY; } - last = tree.execute(last, request, *(void**) client.arg.data, ref ret); + last = tree.execute(last, request, *(void**) client.arg.data, out ret); my_errno = Posix.errno; Posix.errno = 0; if (last != null) diff --git a/tests/test-ioctl-tree.c b/tests/test-ioctl-tree.c index 6356986e..1a5bca1a 100644 --- a/tests/test-ioctl-tree.c +++ b/tests/test-ioctl-tree.c @@ -554,9 +554,9 @@ t_evdev(void) g_assert(memcmp(&bits_query, "\0\0\0\0\xAA\xAA\xAA\xAA", 8) == 0); /* undefined for other ev type */ - g_assert(!ioctl_tree_execute(tree, NULL, EVIOCGBIT(EV_REL, sizeof(synbits)), &bits_query, NULL)); + g_assert(!ioctl_tree_execute(tree, NULL, EVIOCGBIT(EV_REL, sizeof(synbits)), &bits_query, &ret)); /* undefined for other length */ - g_assert(!ioctl_tree_execute(tree, NULL, EVIOCGBIT(EV_KEY, 4), &bits_query, NULL)); + g_assert(!ioctl_tree_execute(tree, NULL, EVIOCGBIT(EV_KEY, 4), &bits_query, &ret)); ioctl_tree_free(tree); } diff --git a/tests/test-umockdev-vala.vala b/tests/test-umockdev-vala.vala index cc939f66..0d4add09 100644 --- a/tests/test-umockdev-vala.vala +++ b/tests/test-umockdev-vala.vala @@ -271,7 +271,7 @@ E: SUBSYSTEM=usb assert_cmpint (Posix.ioctl (fd, Ioctl.USBDEVFS_CONNECTINFO, ref ci), CompareOperator.EQ, -1); // usually ENOTTY, but seem to be EINVAL assert_cmpint (Posix.errno, CompareOperator.GE, 22); - errno = 0; + Posix.errno = 0; // unknown ioctls don't work on an emulated device assert_cmpint (Posix.ioctl (fd, Ioctl.TIOCSBRK, 0), CompareOperator.EQ, -1);