FreeBSD/src
FreeBSD/src
0
4
Code Issues Pull requests 10 Projects Releases Packages Activity Actions 29

virtual_oss(8): Properly cleanup cuse(3) #41

Manually merged
christos merged 2 commits from christos/vosscuse into main 2026-06-11 09:31:27 +00:00 AGit
Conversation 6 Commits 2 Files changed 2 +46 -12
christos commented 2026-05-29 11:42:01 +00:00
Member
Copy link

virtual_oss(8) does not currently keep track of the cuse(3) it creates,
nor does it destroy any of them on exit, except for the control device.
This is harmless if virtual_oss(8) is shutdown after all audio streams
have been shut down, but if it's killed during I/O, the process hangs
and/or goes into uninterruptible sleep state.

To fix this, have pointers to all cuse(3) devices, and explicitly
destroy them on exit.

Sponsored by: The FreeBSD Foundation
MFC after: 1 week

virtual_oss(8) does not currently keep track of the cuse(3) it creates, nor does it destroy any of them on exit, except for the control device. This is harmless if virtual_oss(8) is shutdown after all audio streams have been shut down, but if it's killed during I/O, the process hangs and/or goes into uninterruptible sleep state. To fix this, have pointers to all cuse(3) devices, and explicitly destroy them on exit. Sponsored by: The FreeBSD Foundation MFC after: 1 week
virtual_oss(8): Properly cleanup cuse(3)
Some checks failed
Checklist / commit (pull_request_target) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Style Checker / Style Checker (pull_request) Has been cancelled
Details
a13e2ab020 
virtual_oss(8) does not currently keep track of the cuse(3) it creates,
nor does it destroy any of them on exit, except for the control device.
This is harmless if virtual_oss(8) is shutdown after all audio streams
have been shut down, but if it's killed during I/O, the process hangs
and/or goes into uninterruptible sleep state.

To fix this, have pointers to all cuse(3) devices, and explicitly
destroy them on exit.

Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
christos commented 2026-05-29 11:42:56 +00:00
Author
Member
Copy link

@jrm @0mp This should fix the issues we talked about in #33. Feel free to test.

@jrm @0mp This should fix the issues we talked about in https://ron-dev.freebsd.org/FreeBSD/src/pulls/33. Feel free to test.
christos requested reviews from jrm, 0mp 2026-05-29 11:58:59 +00:00
jrm reviewed 2026-06-04 16:52:05 +00:00
usr.sbin/virtual_oss/virtual_oss/main.c
@ -1957,6 +1958,7 @@ dup_profile(vprofile_t *pvp, int *pamp, int pol, int rx_mute,
free(ptr);
jrm commented 2026-06-04 16:52:05 +00:00
Member
Copy link

Is this a problem?

  1. dup_profile() is called with both pvp->oss_name and pvp->wav_name non-empty.
  2. ptr is allocated and copied
  3. OSS device is created successfully
  4. WAV device creation fails
  5. free(ptr) releases the heap memory, but ptr->oss_dev is discarded and cuse_dev_destroy(ptr->oss_dev) is never called.

Maybe something like this?

	if (pdev == NULL) {
		if (ptr->oss_dev != NULL)
			cuse_dev_destroy(ptr->oss_dev);
		if (ptr->fd_sta >= 0)
			close(ptr->fd_sta);
		free(ptr);
		return ("Could not create CUSE WAV device");
	}
Is this a problem? 1. `dup_profile()` is called with both `pvp->oss_name` and `pvp->wav_name` non-empty. 2. `ptr` is allocated and copied 3. OSS device is created successfully 4. WAV device creation fails 6. `free(ptr)` releases the heap memory, but `ptr->oss_dev` is discarded and `cuse_dev_destroy(ptr->oss_dev)` is never called. Maybe something like this? ``` if (pdev == NULL) { if (ptr->oss_dev != NULL) cuse_dev_destroy(ptr->oss_dev); if (ptr->fd_sta >= 0) close(ptr->fd_sta); free(ptr); return ("Could not create CUSE WAV device"); } ```
christos commented 2026-06-09 12:54:31 +00:00
Author
Member
Copy link

Good catch, will update the PR.

Good catch, will update the PR.
christos marked this conversation as resolved
jrm reviewed 2026-06-04 16:58:13 +00:00
usr.sbin/virtual_oss/virtual_oss/main.c
@ -2648,3 +2653,4 @@
if (voss_ctl_device[0] != 0)
cuse_dev_destroy(pdev);
TAILQ_FOREACH(pvp, &virtual_profile_client_head, entry) {
jrm commented 2026-06-04 16:58:13 +00:00
Member
Copy link

This is a minor nit you can certainly ignore. These two loops are almost identical. Maybe a helper function?

This is a minor nit you can certainly ignore. These two loops are almost identical. Maybe a helper function?
christos marked this conversation as resolved
christos force-pushed christos/vosscuse from a13e2ab020
Some checks failed
Checklist / commit (pull_request_target) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Style Checker / Style Checker (pull_request) Has been cancelled
Details
to d6ae736619
Some checks failed
Checklist / commit (pull_request_target) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Style Checker / Style Checker (pull_request) Has been cancelled
Details
2026-06-09 13:16:58 +00:00 Compare
christos force-pushed christos/vosscuse from d6ae736619
Some checks failed
Checklist / commit (pull_request_target) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Style Checker / Style Checker (pull_request) Has been cancelled
Details
to de80446e0f
Some checks failed
Checklist / commit (pull_request_target) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Style Checker / Style Checker (pull_request) Has been cancelled
Details
2026-06-09 13:39:15 +00:00 Compare
jrm reviewed 2026-06-09 16:48:41 +00:00
usr.sbin/virtual_oss/virtual_oss/main.c
@ -2651,0 +2679,4 @@
}
cuse_uninit();
close(voss_fd_sta);
jrm commented 2026-06-09 16:48:41 +00:00
Member
Copy link

It looks like voss_fd_sta can fall through here and still be -1. Is it conventional to use a guard for something like this?

  if (voss_fd_sta >= 0)
      close(voss_fd_sta);

I'm unsure about our convention. Maybe we're fine calling close() on something that's not an active file descriptor?

It looks like `voss_fd_sta` can fall through here and still be `-1`. Is it conventional to use a guard for something like this? ``` if (voss_fd_sta >= 0) close(voss_fd_sta); ``` I'm unsure about our convention. Maybe we're fine calling `close()` on something that's not an active file descriptor?
christos commented 2026-06-10 10:33:58 +00:00
Author
Member
Copy link

This is harmless, we don't need to check.

This is harmless, we don't need to check.
jrm reviewed 2026-06-09 16:50:42 +00:00
usr.sbin/virtual_oss/virtual_oss/main.c
@ -1949,1 +1956,3 @@
init_sndstat(ptr);
if (voss_fd_sta < 0) {
if ((voss_fd_sta = open("/dev/sndstat", O_WRONLY)) < 0) {
errstr = "Could not open /dev/sndstat";
jrm commented 2026-06-09 16:50:42 +00:00
Member
Copy link

We're changing from a warn to a fatal error here. Should note that in the commit message?

We're changing from a `warn` to a fatal error here. Should note that in the commit message?
christos commented 2026-06-10 10:35:36 +00:00
Author
Member
Copy link

Not really, it's very unlikely anyone can hit this error because /dev/sndstat is always created when sound(4) attaches, but it's good to fail if we do somehow hit an error here.

Not really, it's very unlikely anyone can hit this error because `/dev/sndstat` is always created when `sound(4)` attaches, but it's good to fail if we do somehow hit an error here.
jrm commented 2026-06-10 14:07:10 +00:00
Member
Copy link

lgtm

lgtm
christos force-pushed christos/vosscuse from de80446e0f
Some checks failed
Checklist / commit (pull_request_target) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Style Checker / Style Checker (pull_request) Has been cancelled
Details
to 93a234a694
Some checks failed
Cross-build Kernel / amd64 ubuntu-22.04 (clang-15) (push) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-22.04 (clang-15) (push) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-24.04 (clang-18) (push) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-24.04 (clang-18) (push) Has been cancelled
Details
Cross-build Kernel / amd64 macos-latest (clang-18) (push) Has been cancelled
Details
Cross-build Kernel / aarch64 macos-latest (clang-18) (push) Has been cancelled
Details
Checklist / commit (pull_request_target) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-22.04 (clang-15) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 ubuntu-24.04 (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / amd64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Cross-build Kernel / aarch64 macos-latest (clang-18) (pull_request) Has been cancelled
Details
Style Checker / Style Checker (pull_request) Has been cancelled
Details
2026-06-11 09:30:43 +00:00 Compare
gitadm referenced this pull request from a commit 2026-06-11 09:31:23 +00:00
virtual_oss(8): Make sndstat FD global
christos manually merged commit 93a234a694 into main 2026-06-11 09:31:27 +00:00
gitadm referenced this pull request from a commit 2026-06-18 19:34:01 +00:00
virtual_oss(8): Make sndstat FD global
Sign in to join this conversation.
Reviewers
No reviewers
jrm
0mp
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
FreeBSD/src!41
No description provided.