diff --git a/CMakeLists.txt b/CMakeLists.txt index bbf152a..58090b1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -91,7 +91,7 @@ dmod_add_library(${DMOD_MODULE_NAME} ${DMOD_MODULE_VERSION} # for them. dmod_link_modules(${DMOD_MODULE_NAME} dmfsi - dmdrvi@>=2.6 + dmdrvi dmini dmlist dmosi diff --git a/docs/open-mode-validation.md b/docs/open-mode-validation.md new file mode 100644 index 0000000..b04c10e --- /dev/null +++ b/docs/open-mode-validation.md @@ -0,0 +1,54 @@ +# Device access and mount lifetime validation + +DMFSI represents read/write access as 3, whereas DMDRVI uses 4. dmdevfs now +maps the access bits at its filesystem/driver boundary and preserves the +existing non-access flags. Invalid access mode zero is rejected before calling +the driver. Each test handle records the actual flags received by the driver. + +Validation on 2026-10-16: + +- Native `test_dmdevfs`: 10/10 steps, including the new read, write, read/write, + create/truncate, append and invalid-mode regression. +- Native `test_partitions` passed; STM32F7 build passed. +- STM32F746G-DISCO booted with the changed dmdevfs: RTC 11/11 and RNG 6/6. + The generic rtc CLI sets time through an `r+b` VFS handle successfully. +- Before the lifetime fix below, full native ctest: 7/9 targets passed. `test_dmdevmon` and `test_order` + terminate with SIGSEGV in repeated mount/configuration operations. Both + reproduce with the unchanged `origin/main` dmdevfs source and the same test + environment (exit -11). A GDB capture of test_order places the fault in the + hotplug thread's Dmod_GetDifFunction/Dmod_Context_IsValid with an invalid + module-context pointer. Those failures are not covered by the passing + access-mode regression; the lifetime fix below resolves them. + +The test CMake scope clears inherited DMOD_MAL_IMPLS: the filesystem, rather +than its consumers, implements dmdrvi notification callbacks. Dependencies are +unversioned in CMake; the packager derives required API versions. + +## Mount/worker lifetime fix + +The class-probing worker released g_devfs_mutex after its live-mount lookup, +then accessed the mount's driver nodes and module context without retaining +them. Concurrent filesystem deinit removed the mount from the registry and +freed those objects. Repeated mount/unmount tests therefore raced a real +use-after-free, independently of filesystem open-mode translation. + +Worker operations now retain a mount reference under the registry mutex. +Unmount unregisters it to block new worker references, then sleeps on a +per-mount semaphore until active operations finish before destroying drivers. +No global mutex is held while waiting or invoking driver callbacks. Pending +mount notifications carry a generation so reuse of a freed mount address +cannot make a stale notification operate on a newer mount. Failed mount +initialization follows the same teardown ordering. + +Validation on 2026-10-16: + +- Full native ctest: 10/10, including test_order and test_dmdevmon. +- Both previous failures plus the lifetime regression: 20 consecutive passes + per target (60 executions). +- The new regression pauses a real mock-driver class probe with semaphores, + starts unmount concurrently and verifies the driver is not freed and + unmount cannot complete before releasing the probe. A second mount with a + queued notification can be unmounted while the worker is blocked elsewhere. +- The regression fails against the exact pre-fix dmdevfs source: driver free + and unmount complete prematurely (0/1 steps, exit 1). +- STM32F7 build passed. diff --git a/src/dmdevfs.c b/src/dmdevfs.c index 90b01d5..ca8e2a1 100644 --- a/src/dmdevfs.c +++ b/src/dmdevfs.c @@ -171,6 +171,7 @@ typedef struct dmdrvi_context_t context; dmdrvi_dev_num_t dev_num; bool available; // true: device became available, false: no longer available + uint64_t mount_generation; // Reject queued notifications for a reused mount address. dmfsi_context_t mounted; // Not NULL: report the nodes of this mount instead (see dmfsi_dmdevfs_mounted()) } hotplug_event_t; @@ -242,6 +243,10 @@ struct dmfsi_context uint32_t magic; char* config_path; // Path with the configuration files dmlist_context_t* drivers; // List of loaded drivers + uint64_t generation; + unsigned worker_refs; // Active unlocked worker operations; guarded by g_devfs_mutex. + bool stopping; + dmosi_semaphore_t workers_idle; bool ready; // True once dmfsi_dmdevfs_mounted() has been called by the mounter path_t mount_path; // This mount's own absolute path, valid once `ready` is true }; @@ -255,6 +260,7 @@ struct dmfsi_context * the right mount (and driver_node_t) to update. */ static dmlist_context_t* g_dmdevfs_mounts = NULL; +static uint64_t g_mount_generation; /** * @brief Guards g_dmdevfs_mounts and every mount's ctx->drivers list against @@ -353,7 +359,7 @@ static void hotplug_worker_thread( void* arg ); static bool queue_hotplug_event( dmdrvi_context_t context, const dmdrvi_dev_num_t* dev_num, bool available ); static void process_device_available( dmdrvi_context_t context, const dmdrvi_dev_num_t* dev_num ); static void process_device_unavailable( dmdrvi_context_t context, const dmdrvi_dev_num_t* dev_num ); -static void process_mount_ready( dmfsi_context_t mount ); +static void process_mount_ready( dmfsi_context_t mount, uint64_t generation ); // ============================================================================ // Module Interface Implementation @@ -454,6 +460,39 @@ int dmod_deinit(void) // DMFSI Interface Implementation // ============================================================================ +/* Retain under g_devfs_mutex after a registry lookup, before invoking driver + * code without that mutex. Unmount removes the registry entry first and waits + * for the last active worker to finish before freeing any driver or list. */ +static void retain_worker_mount_locked(dmfsi_context_t mount) +{ + ++mount->worker_refs; +} + +static void release_worker_mount(dmfsi_context_t mount) +{ + dmosi_mutex_lock(g_devfs_mutex); + if (--mount->worker_refs == 0 && mount->stopping) + { + // Post before dropping the lock: unmount may destroy the semaphore + // as soon as its wait completes. Do not access mount after this post. + dmosi_semaphore_post(mount->workers_idle, 1); + } + dmosi_mutex_unlock(g_devfs_mutex); +} + +static void detach_mount_and_wait(dmfsi_context_t mount) +{ + dmosi_mutex_lock(g_devfs_mutex); + dmlist_remove(g_dmdevfs_mounts, mount, compare_mount_context); + mount->stopping = true; + bool busy = mount->worker_refs != 0; + dmosi_mutex_unlock(g_devfs_mutex); + if (busy) + { + dmosi_semaphore_wait(mount->workers_idle, 1, -1); + } +} + /** * @brief Initialize the file system */ @@ -481,6 +520,17 @@ dmod_dmfsi_dif_api_declaration( 1.0, dmdevfs, dmfsi_context_t, _init, (const cha ctx->magic = DMDEVFS_CONTEXT_MAGIC; ctx->config_path = Dmod_StrDup(config); ctx->drivers = dmlist_create(); + ctx->worker_refs = 0; + ctx->stopping = false; + ctx->workers_idle = dmosi_semaphore_create(0, 1); + if (ctx->config_path == NULL || ctx->drivers == NULL || ctx->workers_idle == NULL) + { + if (ctx->workers_idle != NULL) dmosi_semaphore_destroy(ctx->workers_idle); + if (ctx->drivers != NULL) dmlist_destroy(ctx->drivers); + Dmod_Free(ctx->config_path); + Dmod_Free(ctx); + return NULL; + } ctx->ready = false; ctx->mount_path[0] = '\0'; @@ -497,26 +547,21 @@ dmod_dmfsi_dif_api_declaration( 1.0, dmdevfs, dmfsi_context_t, _init, (const cha // mount even while a driver's own dmdrvi_create() is still running as // part of configure_drivers() below. dmosi_mutex_lock(g_devfs_mutex); - bool registered = dmlist_push_back(g_dmdevfs_mounts, ctx); + ctx->generation = g_mount_generation == UINT64_MAX ? 0 : ++g_mount_generation; + bool registered = ctx->generation != 0 && dmlist_push_back(g_dmdevfs_mounts, ctx); dmosi_mutex_unlock(g_devfs_mutex); if (!registered) { DMOD_LOG_ERROR("Failed to register mount for hot-plug notifications\n"); } - int res = configure_drivers(ctx, NULL, ctx->config_path); + int res = registered ? configure_drivers(ctx, NULL, ctx->config_path) : DMFSI_ERR_GENERAL; if (res != DMFSI_OK) { DMOD_LOG_ERROR("Failed to configure drivers\n"); + detach_mount_and_wait(ctx); unconfigure_drivers(ctx); - - if (registered) - { - dmosi_mutex_lock(g_devfs_mutex); - dmlist_remove(g_dmdevfs_mounts, ctx, compare_mount_context); - dmosi_mutex_unlock(g_devfs_mutex); - } - + dmosi_semaphore_destroy(ctx->workers_idle); dmlist_destroy(ctx->drivers); Dmod_Free(ctx->config_path); Dmod_Free(ctx); @@ -545,20 +590,31 @@ dmod_dmfsi_dif_api_declaration( 1.0, dmdevfs, int, _deinit, (dmfsi_context_t ctx return DMFSI_ERR_INVALID; } - // Deregister before tearing down ctx->drivers: once removed, the hot-plug - // worker thread can no longer find (or touch) this mount, so the rest of - // teardown below can safely run without holding g_devfs_mutex. - dmosi_mutex_lock(g_devfs_mutex); - dmlist_remove(g_dmdevfs_mounts, ctx, compare_mount_context); - dmosi_mutex_unlock(g_devfs_mutex); - + detach_mount_and_wait(ctx); unconfigure_drivers(ctx); + dmosi_semaphore_destroy(ctx->workers_idle); dmlist_destroy(ctx->drivers); Dmod_Free(ctx->config_path); Dmod_Free(ctx); return DMFSI_OK; } +/* dmfsi encodes RDWR as RDONLY|WRONLY (3), while dmdrvi has a + * separate RDWR bit (4). Translate at this interface boundary, retaining + * the non-access flags that were already forwarded to drivers. */ +static int driver_open_mode(int mode) +{ + int access = mode & (DMFSI_O_RDONLY | DMFSI_O_WRONLY); + int extra = mode & ~(DMFSI_O_RDONLY | DMFSI_O_WRONLY); + switch (access) + { + case DMFSI_O_RDONLY: return extra | DMDRVI_O_RDONLY; + case DMFSI_O_WRONLY: return extra | DMDRVI_O_WRONLY; + case DMFSI_O_RDWR: return extra | DMDRVI_O_RDWR; + default: return -1; + } +} + /** * @brief Open a file */ @@ -576,6 +632,10 @@ dmod_dmfsi_dif_api_declaration( 1.0, dmdevfs, int, _fopen, (dmfsi_context_t ctx, return DMFSI_ERR_INVALID; } + *fp = NULL; + int driver_mode = driver_open_mode(mode); + if (driver_mode < 0) return DMFSI_ERR_INVALID; + // Find the driver node for this file, taking a reference on it so it // cannot be freed out from under this handle by a concurrent // process_device_unavailable() - see find_and_ref_driver_node(). @@ -616,7 +676,7 @@ dmod_dmfsi_dif_api_declaration( 1.0, dmdevfs, int, _fopen, (dmfsi_context_t ctx, // Open the device through the driver, identifying which device within the // context this is (the context's own device, or a hot-plugged sub-device) - handle->driver_handle = dmdrvi_open(driver_node->driver_context, mode, &driver_node->dev_num); + handle->driver_handle = dmdrvi_open(driver_node->driver_context, driver_mode, &driver_node->dev_num); if(handle->driver_handle == NULL) { DMOD_LOG_ERROR("Driver failed to open device: %s\n", path); @@ -3768,6 +3828,7 @@ dmod_dmfsi_dif_api_declaration( 1.0, dmdevfs, void, _mounted, (dmfsi_context_t c } memset(event, 0, sizeof(*event)); event->mounted = ctx; + event->mount_generation = ctx->generation; dmosi_mutex_lock(g_hotplug_mutex); bool queued = dmlist_push_back(g_hotplug_events, event); dmosi_mutex_unlock(g_hotplug_mutex); @@ -3789,13 +3850,16 @@ dmod_dmfsi_dif_api_declaration( 1.0, dmdevfs, void, _mounted, (dmfsi_context_t c * may need (report_node_ready()). A node reported meanwhile (a hot-plug * event queued earlier) is left alone, its partitions too. */ -static void process_mount_ready( dmfsi_context_t mount ) +static void process_mount_ready( dmfsi_context_t mount, uint64_t generation ) { dmosi_mutex_lock(g_devfs_mutex); bool live = dmlist_find(g_dmdevfs_mounts, mount, compare_mount_context) != NULL; + // A stale queued pointer may equal the address of a later mount. + live = live && mount->generation == generation; path_t mount_path; if (live) { + retain_worker_mount_locked(mount); strncpy(mount_path, mount->mount_path, sizeof(mount_path)); mount_path[sizeof(mount_path) - 1] = '\0'; } @@ -3821,6 +3885,7 @@ static void process_mount_ready( dmfsi_context_t mount ) report_node_ready(mount_path, node); scan_partitions(mount, mount_path, node); } + release_worker_mount(mount); } /** @@ -3853,7 +3918,7 @@ static void hotplug_worker_thread( void* arg ) } if (event->mounted != NULL) { - process_mount_ready(event->mounted); + process_mount_ready(event->mounted, event->mount_generation); Dmod_Free(event); continue; } @@ -3887,9 +3952,9 @@ static void hotplug_worker_thread( void* arg ) * (stable, pointer-only) fields needed to build the new node. * 2. Unlocked: allocate the node and compute its path (may log). * 3. Locked again: re-validate the mount is still registered - if _deinit() - * raced us and won, mount may now be a dangling pointer, so this check - * must only ever compare it (never dereference it) until proven live - - * and only then insert the node. + * started unmounting, discard the new node instead. A worker reference + * keeps the owner, driver module and mount alive across all unlocked + * work until release_worker_mount(), including error exits. */ static void process_device_available( dmdrvi_context_t context, const dmdrvi_dev_num_t* dev_num ) { @@ -3911,6 +3976,7 @@ static void process_device_available( dmdrvi_context_t context, const dmdrvi_dev return; } + retain_worker_mount_locked(mount); driver_node_t* owner = (driver_node_t*)dmlist_find(mount->drivers, (void*)context, compare_driver_context); dmdrvi_context_t owner_context = owner->driver_context; Dmod_Context_t* owner_driver = owner->driver; @@ -3927,6 +3993,7 @@ static void process_device_available( dmdrvi_context_t context, const dmdrvi_dev DMOD_LOG_ERROR("dmdrvi_device_available: failed to retain friends metadata\n"); if (owner_role != NULL) Dmod_Free(owner_role); if (owner_group != NULL) Dmod_Free(owner_group); + release_worker_mount(mount); return; } @@ -3936,6 +4003,7 @@ static void process_device_available( dmdrvi_context_t context, const dmdrvi_dev DMOD_LOG_ERROR("dmdrvi_device_available: failed to allocate device node\n"); if (owner_role != NULL) Dmod_Free(owner_role); if (owner_group != NULL) Dmod_Free(owner_group); + release_worker_mount(mount); return; } @@ -3961,6 +4029,7 @@ static void process_device_available( dmdrvi_context_t context, const dmdrvi_dev { DMOD_LOG_ERROR("dmdrvi_device_available: failed to compute device path\n"); free_driver_node(new_node); + release_worker_mount(mount); return; } @@ -3982,12 +4051,14 @@ static void process_device_available( dmdrvi_context_t context, const dmdrvi_dev { DMOD_LOG_ERROR("dmdrvi_device_available: mount was unmounted while processing event\n"); free_driver_node(new_node); + release_worker_mount(mount); return; } if (!inserted) { DMOD_LOG_ERROR("dmdrvi_device_available: failed to register device node\n"); free_driver_node(new_node); + release_worker_mount(mount); return; } @@ -4003,6 +4074,7 @@ static void process_device_available( dmdrvi_context_t context, const dmdrvi_dev } DMOD_LOG_INFO("Hot-plugged device now available: %s\n", new_node->path); + release_worker_mount(mount); } /** @@ -4024,6 +4096,7 @@ static void process_device_unavailable( dmdrvi_context_t context, const dmdrvi_d return; } + retain_worker_mount_locked(mount); driver_node_t* node = (driver_node_t*)dmlist_find(mount->drivers, &lookup, compare_dynamic_device); path_t removed_path; strncpy(removed_path, node->path, sizeof(removed_path)); @@ -4054,4 +4127,5 @@ static void process_device_unavailable( dmdrvi_context_t context, const dmdrvi_d release_driver_node_ref(node); DMOD_LOG_INFO("Hot-plugged device no longer available: %s\n", removed_path); + release_worker_mount(mount); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 02fce28..99974ef 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -2,6 +2,9 @@ cmake_minimum_required(VERSION 3.18) project(dmdevfs_tests VERSION 1.0 DESCRIPTION "DMDEVFS Unit Tests" LANGUAGES C) +# Tests consume dmdevfs; only the filesystem implements the driver MAL. +set(DMOD_MAL_IMPLS "") + # Ensure DMOD is available if(NOT DMOD_DIR) message(FATAL_ERROR "DMOD_DIR not set. Tests must be built as part of main project.") @@ -98,7 +101,8 @@ target_compile_definitions(${DMOD_MODULE_NAME} PRIVATE ) # dmdrvi: DMDRVI_IOCTL_DEVFS_GET_FRIEND (2.4), DMDRVI_IOCTL_AUDIO_* (2.6) dmod_link_modules(${DMOD_MODULE_NAME} - dmdrvi@>=2.6 + dmdrvi + dmini dmfsi dmhaman dmosi @@ -124,7 +128,7 @@ target_compile_definitions(${DMOD_MODULE_NAME} PRIVATE ) dmod_link_modules(${DMOD_MODULE_NAME} dmfsi - dmdrvi@>=2.6 + dmdrvi dmhaman dmosi ) @@ -143,7 +147,7 @@ target_compile_definitions(${DMOD_MODULE_NAME} PRIVATE ) dmod_link_modules(${DMOD_MODULE_NAME} dmfsi - dmdrvi@>=2.6 + dmdrvi dmhaman dmosi libsystemd @@ -163,13 +167,20 @@ target_compile_definitions(${DMOD_MODULE_NAME} PRIVATE ) dmod_link_modules(${DMOD_MODULE_NAME} dmfsi - dmdrvi@>=2.6 + dmdrvi dmhaman dmosi libsystemd ) dmdevfs_register_test(${DMOD_MODULE_NAME}) +# Mount teardown while asynchronous node probing is in flight. +set(DMOD_MODULE_NAME test_mount_lifetime) +dmod_add_test(${DMOD_MODULE_NAME} ${DMOD_MODULE_VERSION} test_mount_lifetime.c) +target_include_directories(${DMOD_MODULE_NAME} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/mockdrv) +dmod_link_modules(${DMOD_MODULE_NAME} dmfsi dmosi) +dmdevfs_register_test(${DMOD_MODULE_NAME}) + # Print information about tests message(STATUS "DMDEVFS tests configured") message(STATUS " Module output directory: ${CMAKE_BINARY_DIR}/dmf") diff --git a/tests/fixtures/lifetime/mock.ini b/tests/fixtures/lifetime/mock.ini new file mode 100644 index 0000000..46b717c --- /dev/null +++ b/tests/fixtures/lifetime/mock.ini @@ -0,0 +1,3 @@ +[main] +driver_name=dmdevfs_mockdrv +major=0 diff --git a/tests/mockdrv/CMakeLists.txt b/tests/mockdrv/CMakeLists.txt index f1d7dd8..c32e6b7 100644 --- a/tests/mockdrv/CMakeLists.txt +++ b/tests/mockdrv/CMakeLists.txt @@ -18,7 +18,8 @@ target_include_directories(${DMOD_MODULE_NAME}_if INTERFACE ) dmod_link_modules(${DMOD_MODULE_NAME} - dmdrvi@>=2.6 + dmdrvi dmini dmlist + dmosi ) diff --git a/tests/mockdrv/dmdevfs_mockdrv.h b/tests/mockdrv/dmdevfs_mockdrv.h index 08a3206..9037928 100644 --- a/tests/mockdrv/dmdevfs_mockdrv.h +++ b/tests/mockdrv/dmdevfs_mockdrv.h @@ -12,12 +12,22 @@ #define DMDEVFS_MOCKDRV_IOCTL_SET_MEDIA 0x1003 /* arg: const uint64_t* block count - only while unplugged; clears the medium */ #define DMDEVFS_MOCKDRV_IOCTL_WRITE_SECTOR 0x1004 /* arg: const dmdevfs_mockdrv_sector_t* - raw write, plugged or not */ +#define DMDEVFS_MOCKDRV_IOCTL_GET_OPEN_FLAGS 0x1005 /* arg: uint32_t* - exact flags passed to this handle */ + /** Logical block size of the child's medium. */ #define DMDEVFS_MOCKDRV_BLOCK_SIZE 512u #include /** Monitor contract calls a monitored host node received so far, and opens of its nodes. */ +#define DMDEVFS_MOCKDRV_IOCTL_PROBE_GATE 0x1006 +/* Test-owned semaphore handles; remain alive until the driver is freed. */ +typedef struct { + void* entered; + void* release; + void* freed; +} dmdevfs_mockdrv_probe_gate_t; + typedef struct { uint32_t events; /**< DMDRVI_IOCTL_MONITOR_EVENT */ diff --git a/tests/mockdrv/mockdrv.c b/tests/mockdrv/mockdrv.c index b27a65d..55f6f0b 100644 --- a/tests/mockdrv/mockdrv.c +++ b/tests/mockdrv/mockdrv.c @@ -3,6 +3,7 @@ #include "dmdrvi.h" #include "dmini.h" #include "dmlist.h" +#include "dmosi.h" #include "dmdevfs_mockdrv.h" #include #include @@ -33,6 +34,7 @@ struct dmdrvi_context { + dmdevfs_mockdrv_probe_gate_t probe_gate; uint32_t magic; uint8_t major; bool monitor; @@ -54,6 +56,7 @@ typedef struct { uint32_t magic; bool is_child; + uint32_t open_flags; } mockdrv_handle_t; /* Contexts created so far - only ever grows, so create_seq orders them. */ @@ -212,6 +215,7 @@ dmod_dmdrvi_dif_api_declaration(2.0, dmdevfs_mockdrv, dmdrvi_context_t, _create, return NULL; } const char* monitor = dmini_get_string(config, NULL, "monitor", "false"); + memset(&ctx->probe_gate, 0, sizeof(ctx->probe_gate)); ctx->magic = MOCKDRV_CONTEXT_MAGIC; ctx->major = (uint8_t)dmini_get_int(config, NULL, "major", 0); ctx->monitor = (strcmp(monitor, "true") == 0); @@ -242,6 +246,8 @@ dmod_dmdrvi_dif_api_declaration(2.0, dmdevfs_mockdrv, void, _free, ( dmdrvi_cont { if (is_valid(context)) { + if (context->probe_gate.freed != NULL) + dmosi_semaphore_post(context->probe_gate.freed, 1); clear_medium(context); dmlist_destroy(context->sectors); context->magic = 0; @@ -263,6 +269,7 @@ dmod_dmdrvi_dif_api_declaration(2.0, dmdevfs_mockdrv, void*, _open, { handle->magic = MOCKDRV_HANDLE_MAGIC; handle->is_child = is_child; + handle->open_flags = (uint32_t)flags; if (is_child) { context->child_opens++; @@ -348,6 +355,19 @@ static int monitor_ioctl(dmdrvi_context_t context, int command, void* arg) static int host_ioctl(dmdrvi_context_t context, int command, void* arg) { + if (command == DMDEVFS_MOCKDRV_IOCTL_PROBE_GATE) { + if (arg == NULL) return -EINVAL; + context->probe_gate = *(const dmdevfs_mockdrv_probe_gate_t*)arg; + return 0; + } + if (command == DMDRVI_IOCTL_MONITOR_GET_POLICY && context->probe_gate.entered != NULL) { + void* entered = context->probe_gate.entered; + void* release = context->probe_gate.release; + context->probe_gate.entered = NULL; + dmosi_semaphore_post(entered, 1); + int rc = dmosi_semaphore_wait(release, 1, 5000); + if (rc != 0) return rc; + } switch (command) { case DMDRVI_IOCTL_MONITOR_GET_POLICY: @@ -430,6 +450,11 @@ dmod_dmdrvi_dif_api_declaration(2.0, dmdevfs_mockdrv, int, _ioctl, { return -EINVAL; } + if (command == DMDEVFS_MOCKDRV_IOCTL_GET_OPEN_FLAGS) { + if (arg == NULL) return -EINVAL; + *(uint32_t*)arg = h->open_flags; + return 0; + } if (!h->is_child) { return host_ioctl(context, command, arg); diff --git a/tests/test_dmdevfs.c b/tests/test_dmdevfs.c index a70a0a4..c319805 100644 --- a/tests/test_dmdevfs.c +++ b/tests/test_dmdevfs.c @@ -6,7 +6,7 @@ #include "dmhaman.h" #include "libsystemd.h" #include "dmdevfs_mockdrv.h" -#include "dmdrvi_ioctl.h" +#include "dmdrvi.h" #include #include @@ -326,3 +326,26 @@ DMOD_TEST_STEP(dmdevfs_reports_removal_of_plugged_node_on_teardown) DMOD_TEST_EXPECT_TRUE(wait_state("blk@dmdevfs_mockdrv2_0", DMOSI_PROCESS_STATE_TERMINATED)); DMOD_TEST_EXPECT_TRUE(wait_state("mon@dmdevfs_mockdrv0", DMOSI_PROCESS_STATE_TERMINATED)); } + +DMOD_TEST_STEP(device_access_modes_are_translated_at_the_filesystem_boundary) +{ + DMOD_TEST_EXPECT_NOT_NULL(g_mount); + if (g_mount == NULL) return; + const int modes[] = {DMFSI_O_RDONLY, DMFSI_O_WRONLY, DMFSI_O_RDWR, + DMFSI_O_WRONLY | DMFSI_O_CREAT | DMFSI_O_TRUNC, DMFSI_O_RDWR | DMFSI_O_APPEND}; + const int expected[] = {DMDRVI_O_RDONLY, DMDRVI_O_WRONLY, DMDRVI_O_RDWR, + DMDRVI_O_WRONLY | DMFSI_O_CREAT | DMFSI_O_TRUNC, DMDRVI_O_RDWR | DMFSI_O_APPEND}; + for (size_t i = 0; i < sizeof(modes)/sizeof(modes[0]); ++i) { + void* file = NULL; uint32_t flags = 0; + DMOD_TEST_EXPECT_EQ(g_fs.fopen(g_mount, &file, "/dmdevfs_mockdrv2", modes[i], 0), DMFSI_OK); + DMOD_TEST_EXPECT_NOT_NULL(file); + if (file != NULL) { + DMOD_TEST_EXPECT_EQ(g_fs.ioctl(g_mount, file, DMDEVFS_MOCKDRV_IOCTL_GET_OPEN_FLAGS, &flags), 0); + DMOD_TEST_EXPECT_EQ(flags, (uint32_t)expected[i]); + DMOD_TEST_EXPECT_EQ(g_fs.fclose(g_mount, file), DMFSI_OK); + } + } + void* invalid = (void*)1; + DMOD_TEST_EXPECT_EQ(g_fs.fopen(g_mount, &invalid, "/dmdevfs_mockdrv2", 0, 0), DMFSI_ERR_INVALID); + DMOD_TEST_EXPECT_NULL(invalid); +} diff --git a/tests/test_mount_lifetime.c b/tests/test_mount_lifetime.c new file mode 100644 index 0000000..f8e64d5 --- /dev/null +++ b/tests/test_mount_lifetime.c @@ -0,0 +1,89 @@ +#define DMOD_ENABLE_REGISTRATION ON +#define ENABLE_DIF_REGISTRATIONS ON +#include "dmod_test.h" +#include "dmfsi.h" +#include "dmosi.h" +#include "dmdevfs_mockdrv.h" +#include + +static dmod_dmfsi_init_t mount_init; +static dmod_dmfsi_deinit_t mount_deinit; +static dmod_dmfsi_mounted_t mounted; +static dmod_dmfsi_fopen_t file_open; +static dmod_dmfsi_fclose_t file_close; +static dmod_dmfsi_ioctl_t file_ioctl; +static dmfsi_context_t mount; +static dmosi_semaphore_t started, finished; +static int unmount_result; + +static bool load(void) +{ + if (Dmod_LoadModuleByName("dmdevfs") == NULL || !Dmod_EnableModule("dmdevfs", false, NULL)) return false; + Dmod_Context_t* m = Dmod_GetModuleContext("dmdevfs"); + mount_init = Dmod_GetDifFunction(m, dmod_dmfsi_init_sig); + mount_deinit = Dmod_GetDifFunction(m, dmod_dmfsi_deinit_sig); + mounted = Dmod_GetDifFunction(m, dmod_dmfsi_mounted_sig); + file_open = Dmod_GetDifFunction(m, dmod_dmfsi_fopen_sig); + file_close = Dmod_GetDifFunction(m, dmod_dmfsi_fclose_sig); + file_ioctl = Dmod_GetDifFunction(m, dmod_dmfsi_ioctl_sig); + return mount_init && mount_deinit && mounted && file_open && file_close && file_ioctl; +} +static void unmount_thread(void* arg) +{ + (void)arg; + dmosi_semaphore_post(started, 1); + unmount_result = mount_deinit(mount); + dmosi_semaphore_post(finished, 1); +} + +DMOD_TEST_STEP(unmount_keeps_drivers_alive_until_worker_probe_returns) +{ + DMOD_TEST_EXPECT_TRUE(load()); + if (!mount_init || !mount_deinit) return; + dmdevfs_mockdrv_probe_gate_t gate = { + dmosi_semaphore_create(0, 1), dmosi_semaphore_create(0, 1), dmosi_semaphore_create(0, 1)}; + started = dmosi_semaphore_create(0, 1); + finished = dmosi_semaphore_create(0, 1); + DMOD_TEST_EXPECT_NOT_NULL(gate.entered); DMOD_TEST_EXPECT_NOT_NULL(gate.release); + DMOD_TEST_EXPECT_NOT_NULL(gate.freed); DMOD_TEST_EXPECT_NOT_NULL(started); + DMOD_TEST_EXPECT_NOT_NULL(finished); + mount = mount_init("fixtures/lifetime"); + DMOD_TEST_EXPECT_NOT_NULL(mount); + if (mount == NULL) return; + void* file = NULL; + DMOD_TEST_EXPECT_EQ(file_open(mount, &file, "/dmdevfs_mockdrv0", DMFSI_O_RDONLY, 0), DMFSI_OK); + DMOD_TEST_EXPECT_NOT_NULL(file); + if (file == NULL) { mount_deinit(mount); return; } + DMOD_TEST_EXPECT_EQ(file_ioctl(mount, file, DMDEVFS_MOCKDRV_IOCTL_PROBE_GATE, &gate), 0); + file_close(mount, file); + mounted(mount, "/dev"); + DMOD_TEST_EXPECT_EQ(dmosi_semaphore_wait(gate.entered, 1, 2000), 0); + + // A second mount's notification remains queued behind the blocked probe. + // Unmount must cancel it without waiting for this unrelated worker. + dmfsi_context_t queued = mount_init("fixtures/lifetime"); + DMOD_TEST_EXPECT_NOT_NULL(queued); + if (queued != NULL) { + mounted(queued, "/dev/queued"); + DMOD_TEST_EXPECT_EQ(mount_deinit(queued), DMFSI_OK); + } + + dmosi_thread_t thread = dmosi_thread_create(unmount_thread, NULL, 1, 16384, "unmount", NULL); + DMOD_TEST_EXPECT_NOT_NULL(thread); + if (thread != NULL) { + DMOD_TEST_EXPECT_EQ(dmosi_semaphore_wait(started, 1, 2000), 0); + DMOD_TEST_EXPECT_EQ(dmosi_semaphore_wait(finished, 1, 50), -ETIMEDOUT); + DMOD_TEST_EXPECT_EQ(dmosi_semaphore_wait(gate.freed, 1, 10), -ETIMEDOUT); + } + dmosi_semaphore_post(gate.release, 1); + if (thread != NULL) { + DMOD_TEST_EXPECT_EQ(dmosi_semaphore_wait(finished, 1, 2000), 0); + DMOD_TEST_EXPECT_EQ(dmosi_thread_join(thread), 0); + dmosi_thread_destroy(thread); + DMOD_TEST_EXPECT_EQ(unmount_result, DMFSI_OK); + } else mount_deinit(mount); + DMOD_TEST_EXPECT_EQ(dmosi_semaphore_wait(gate.freed, 1, 2000), 0); + dmosi_semaphore_destroy(gate.entered); dmosi_semaphore_destroy(gate.release); + dmosi_semaphore_destroy(gate.freed); dmosi_semaphore_destroy(started); + dmosi_semaphore_destroy(finished); +}