Date: Wed, 15 Jul 2026 12:26:36 +0000 From: bugzilla-noreply@freebsd.org To: bugs@FreeBSD.org Subject: =?UTF-8?B?W0J1ZyAyOTYyOTFdIGN1c2UoMyk6IGN1c2Vfc2VydmVyX2ZyZWUo?= =?UTF-8?B?KSBidXN5LXdhaXRzIHVua2lsbGFibHkgKHBhdXNlKCJXIikpIHdoZW4gYSBj?= =?UTF-8?B?bGllbnQgcmVmICAgaXMgbm90IHJlbGVhc2VkIG9uIHZpcnR1YWxfb3NzIHRl?= =?UTF-8?B?YXJkb3duIOKAlCBwcm9jZXNzIHN0dWNrIGluIEQsIGN1c2Uua28gcGlubmVk?= =?UTF-8?B?LCByZWJvb3QgcmVxdWlyZWQ=?= Message-ID: <bug-296291-227-4hVouBf8ip@https.bugs.freebsd.org/bugzilla/> In-Reply-To: <bug-296291-227@https.bugs.freebsd.org/bugzilla/>
index | next in thread | previous in thread | raw e-mail
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=296291 --- Comment #5 from delleceste@gmail.com --- (In reply to delleceste from comment #4) I don't have a -CURRENT machine, but I can still answer with some confidence, because before your commit landed I had independently written the same fix and compared the two side by side. Your 0bd5ef6b4363 ("virtual_oss(8): Properly cleanup cuse(3)") and my local patch are functionally identical: - same two fields added to struct virtual_profile in int.h (struct cuse_dev *oss_dev / *wav_dev); - same assignments in dup_profile() after cuse_dev_create(); - same teardown in main(): walk virtual_profile_client_head and virtual_profile_loopback_head and cuse_dev_destroy() every DSP/WAV device, instead of destroying only the CTL device. The only differences are cosmetic or extra hygiene on your side: you destroy the devices after destroy_threads() while I did it before (doesn't matter -- cuse_dev_destroy() is an ioctl from the main thread, and the kernel's cuse_server_free_dev() marks the clients closing without needing a worker thread), you also call cuse_uninit(), and you fix the dup_profile() error-path memory leak. Since the change is userland-only and dup_profile()/main() are identical in releng/15.1, I can apply your commit onto a 15.1 build of /usr/sbin/virtual_oss and run it through the reproducer here (the DRC loopback chain with brutefir on dsp.loop, which made the hang ~100% reproducible). That exercises exactly the code you committed, just on a 15.1 kernel. I'll report back with the result. For the moment, since I applied "my" version of the patch I haven't been running into the issue any more, even though extensive or aggressive tests haven't been performed yet. But that is a good sign. Two follow-up points from the kernel-side capture: 1. Your patch mitigates the common trigger (once the devfs nodes are destroyed, new opens fail before reaching cuse_client_open()), but the kernel ref leak stays latent: it does nothing for "kill -9 virtual_oss" (no cleanup runs) or any path that sets is_closing while a node still exists. 2. The leak itself appears to be a regression from 634e578ac7b0 ("cuse: Fix cdevpriv bugs in cuse_client_open()", D53708), which releng/15.1 carries. Before it, the is_closing error path ran devfs_clear_cdevpriv(), which synchronously executed the cuse_client_free() destructor (TAILQ_REMOVE + cuse_server_unref), so the reference was released. Moving devfs_set_cdevpriv() after the is_closing check fixed the half-constructed- client panics, but left both error returns (is_closing and devfs_set_cdevpriv failure) with no cleanup: the server ref taken at the top and the client linked on pcs->hcli now leak on every open() that races the teardown window, and cuse_server_free() spins in pause("W", hz) forever. Proposed fix (patch attached): call cuse_client_free(pcc) directly on both error paths. At those points the client is fully constructed and linked on pcs->hcli -- exactly the state the destructor expects (and for a fresh client cuse_client_is_closing() is a flag-set no-op, since no commands are queued). This restores the pre-634e578ac7b0 semantics without reintroducing the panic paths it fixed. -- You are receiving this mail because: You are the assignee for the bug.home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-296291-227-4hVouBf8ip>