From 79a3eb211ede07ef20b6ee037206b60ccffe1560 Mon Sep 17 00:00:00 2001 From: crux161 Date: Tue, 5 May 2026 10:38:55 -0700 Subject: [PATCH 1/2] Fix use-after-free in UMS Filesystem name/mount_name Filesystem stored name/mount_name as std::string_view referencing strings owned by UmsController::Device. libusbhsfs's populate callback clears the devices vector on every event (partition discovery, mount/unmount), invalidating those views. Subsequent file access via mount_name.data() triggered a use-after-free and crashed when accessing USB storage. Make Filesystem own its strings (std::string). Also fix iterator invalidation in ums_devices_changed_cb where std::erase ran on the vector being range-iterated; switch to std::erase_if and only reset cur_fs if it actually pointed to the removed device. --- src/fs/fs_common.hpp | 2 +- src/main.cpp | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/fs/fs_common.hpp b/src/fs/fs_common.hpp index d3d93af..6b395f6 100644 --- a/src/fs/fs_common.hpp +++ b/src/fs/fs_common.hpp @@ -165,7 +165,7 @@ class Filesystem { public: Type type; - std::string_view name, mount_name; + std::string name, mount_name; protected: devoptab_t devoptab = {}; diff --git a/src/main.cpp b/src/main.cpp index 3c79943..9698567 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -141,19 +141,25 @@ void ums_devices_changed_cb(const std::vector &de auto &context = *static_cast(user); // Remove unmounted devices - for (auto &fs: context.filesystems) { + bool removed_cur = false; + std::erase_if(context.filesystems, [&](const auto &fs) { if (fs->type != sw::fs::Filesystem::Type::Usb) - continue; + return false; auto it = std::find_if(devices.begin(), devices.end(), [&fs](const auto &dev) { return fs->mount_name == dev.mount_name; }); if (it == devices.end()) { - std::erase(context.filesystems, fs); - context.cur_fs = context.filesystems.front(); + if (context.cur_fs == fs) + removed_cur = true; + return true; } - } + return false; + }); + + if (removed_cur && !context.filesystems.empty()) + context.cur_fs = context.filesystems.front(); // Add new devices for (auto &dev: devices) { From caaddf36a8655b3218b07b0c811dc80ae2765eba Mon Sep 17 00:00:00 2001 From: crux161 Date: Tue, 5 May 2026 11:33:28 -0700 Subject: [PATCH 2/2] Defer UMS device-list updates to the main thread libusbhsfs's populate callback runs on its USB-event thread. The previous code mutated context.filesystems / cur_fs directly from that thread, racing with main-thread reads from UI code. Stash the new device list under a mutex; have the main loops (menu_loop, video_loop) drain and apply pending changes once per frame so context.filesystems stays single-threaded for readers. Also handle USB unplug during playback: when the filesystem hosting the currently-playing file disappears, async-stop mpv and break out of video_loop with EIO so the user falls back to the menu cleanly. (Note: this does not eliminate crashes when mpv's demuxer thread is mid-read at the moment of physical removal -- that race is inside libusbhsfs and would require upstream changes to fix.) --- src/main.cpp | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index 9698567..1503de6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -15,9 +15,12 @@ // You should have received a copy of the GNU General Public License // along with SwitchWave. If not, see . +#include #include #include #include +#include +#include #include #include @@ -137,8 +140,26 @@ void mpv_presetup() { std::printf("Dumped standard font\n"); } +// libusbhsfs invokes the populate callback on its USB-event thread. +// Stash the new device list under a mutex; the main thread applies +// the change so context.filesystems / cur_fs stay single-threaded. +std::mutex g_ums_pending_mtx; +std::optional> g_ums_pending; + void ums_devices_changed_cb(const std::vector &devices, void *user) { - auto &context = *static_cast(user); + auto lk = std::scoped_lock(g_ums_pending_mtx); + g_ums_pending = devices; +} + +void apply_pending_ums_changes(sw::Context &context) { + std::vector devices; + { + auto lk = std::scoped_lock(g_ums_pending_mtx); + if (!g_ums_pending) + return; + devices = std::move(*g_ums_pending); + g_ums_pending.reset(); + } // Remove unmounted devices bool removed_cur = false; @@ -187,6 +208,8 @@ int menu_loop(sw::Renderer &renderer, sw::Context &context) { break; } + apply_pending_ums_changes(context); + padUpdate(&g_pad); auto has_touches = hidGetTouchScreenStates(&g_touch_state, 1); ImGui::nx::newFrame(&g_pad, has_touches ? &g_touch_state : nullptr); @@ -283,6 +306,18 @@ int video_loop(sw::Renderer &renderer, sw::Context &context) { break; } + apply_pending_ums_changes(context); + + // If the filesystem hosting the playing file was unplugged + // (e.g. USB key removed mid-playback), stop mpv and bail out + // of the player loop so it can't keep reading a dead device. + if (!context.cur_file.empty() && + !context.get_filesystem(sw::fs::Path::mountpoint(context.cur_file))) { + lmpv.command_async("stop"); + context.set_error(-EIO); + break; + } + padUpdate(&g_pad); auto has_touches = hidGetTouchScreenStates(&g_touch_state, 1); ImGui::nx::newFrame(&g_pad, has_touches ? &g_touch_state : nullptr);