Page MenuHomeFreeBSD
Differential D51865

libusb: capsicumize libusb
Needs ReviewPublic
Actions

Authored by aokblast on Aug 11 2025, 3:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jul 13, 1:19 PM
Unknown Object (File)
Sat, Jul 11, 8:27 PM
Unknown Object (File)
Thu, Jul 9, 11:11 AM
Unknown Object (File)
Tue, Jul 7, 3:48 AM
Unknown Object (File)
Mon, Jul 6, 9:12 PM
Unknown Object (File)
Sun, Jul 5, 7:32 AM
Unknown Object (File)
Thu, Jun 25, 4:40 PM
Unknown Object (File)
Thu, Jun 25, 3:27 PM
View All Files
Subscribers
adrian
imp
jonathan
kevans
ziaee

Details

Reviewers
jfree
pjd
markj
oshogbo
lwhsu
Group Reviewers
capsicum
Summary

FreeBSD's libusb has three components: libusb01, libusb10, and libusb20.

libusb20 handles communication with character devices. We now accept
file descriptors (FDs) for /dev/usb (usbd_fd) and /dev/usbctl (cfd)
directly, allowing users to open these and apply capabilities
themselves.

libusb10 is updated to support capabilities via a context option. Since
libusb allows general read/write access, we preserve all possible
capabilities when passing FDs to libusb20. It's the responsibility of
the libusb user to call cap_enter() at an appropriate time.

libusb01 is currently unused, so Capsicum support is not implemented for
it.

All base system tools using libusb20 have been updated to support
Capsicum.

Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 66192
Build 63075: arc lint + arc unit

Event Timeline

aokblast created this revision.Aug 11 2025, 3:54 PM
Herald added subscribers: ziaee, jonathan, imp. · View Herald TranscriptAug 11 2025, 3:54 PM
aokblast requested review of this revision.Aug 11 2025, 3:54 PM
Harbormaster completed remote builds in B66173: Diff 160192.Aug 11 2025, 3:54 PM
aokblast updated this revision to Diff 160193.Aug 11 2025, 4:02 PM

Don't reorder libusb.

Harbormaster completed remote builds in B66174: Diff 160193.Aug 11 2025, 4:02 PM
aokblast added a reviewer: jfree.Aug 11 2025, 4:03 PM
aokblast added a reviewer: pjd.
aokblast added a reviewer: capsicum.Aug 11 2025, 4:28 PM
lwhsu added reviewers: markj, oshogbo, lwhsu.Aug 11 2025, 4:34 PM
aokblast added a parent revision: D51224: libusb: refactor the process of setting option when init the context..Aug 11 2025, 4:39 PM
jfree added inline comments.Aug 11 2025, 5:34 PM
tools/tools/usbtest/usb_msc_test.c
970

Is this just general unused variable cleanup? I tend to do these small unrelated cleanups in separate patches. This way, the small cleanup gets its own commit and stays if your bigger patch needs to be reverted for whatever reason.

usr.sbin/usbconfig/usbconfig.c
644

In the rest of your patch, you opened usbd_fd as a normal descriptor with O_RDONLY and now you're opening it as a directory descriptor. Maybe the rest of the usbd_fd open() calls should be updated to use O_PATH | O_DIRECTORY and I think you could eliminate most of the capabilities in your rights list for usbd_fd since I only see you using it for openat().

aokblast updated this revision to Diff 160204.Aug 12 2025, 6:22 AM

Fix comments

Harbormaster completed remote builds in B66179: Diff 160204.Aug 12 2025, 6:22 AM
aokblast added inline comments.Aug 12 2025, 6:24 AM
usr.sbin/usbconfig/usbconfig.c
644

The fd returns from the openat will inherit all of the capabilities from the fd pass to openat. Therefore, for the /dev/usb/*.*.* devices, we should give the usbd_fd, READ, WRITE, EVENT, IOCTL for later usage.

jfree added a comment.Aug 12 2025, 1:47 PM

Good to see more Capsicumization efforts going on. I just had a few pedantic suggestions. Otherwise, this generally looks good.

lib/libusb/libusb.3
904

I'd add a .Xr capsicum 4 here since you're mentioning it earlier.

usr.sbin/usbconfig/usbconfig.c
48–49

These extern declarations aren't really doing anything since you declare both variables right below.

The extern declarations in dump.c should be enough. You could move them into a header and include that if you anticipate other source files needing to access them as well.

644

Oops, you're definitely right. Looks good to me then :)

aokblast updated this revision to Diff 160244.Aug 12 2025, 2:39 PM

Fix incorrect close fd for libusb

Harbormaster completed remote builds in B66192: Diff 160244.Aug 12 2025, 2:39 PM
aokblast updated this revision to Diff 160245.Aug 12 2025, 2:52 PM

Fix comments

Harbormaster completed remote builds in B66193: Diff 160245.Aug 12 2025, 2:52 PM
aokblast added inline comments.Aug 12 2025, 2:52 PM
usr.sbin/usbconfig/usbconfig.c
48–49

You are right, I am too lazy:). Fix it now.

aokblast updated this revision to Diff 160587.Aug 19 2025, 11:29 AM

recover status when capsicum failed

Harbormaster completed remote builds in B66341: Diff 160587.Aug 19 2025, 11:29 AM
aokblast added a child revision: D52166: bhyve: implement single USB device passthrough support.Aug 26 2025, 4:04 PM
aokblast updated this revision to Diff 179607.Jun 11 2026, 3:02 PM

Rebase and update

Harbormaster completed remote builds in B73824: Diff 179607.Jun 11 2026, 3:02 PM
adrian mentioned this in D52166: bhyve: implement single USB device passthrough support.Sun, Jul 12, 2:48 PM
adrian added a subscriber: adrian.Sun, Jul 12, 3:08 PM

hi!

So why are a whole lot of functions getting an fd added to them?

Do the FDs change during the runtime of the tool? Or are they something that could be set
once during some init phase (maybe in a new function which would call cap_enter, rather than
the owner having to call it) and then pulled out of some the libusb backend state?

kevans added a subscriber: kevans.Sun, Jul 12, 3:12 PM
kevans added inline comments.
lib/libusb/libusb.3
125

I'd maybe re-word this slightly to something like:

Prepare
.Nm
for operation within a sandbox.
Resources that will be needed for regular operation are pre-opened
with appropriate
.Xr rights 4
applied.
Note that it is users' responsiblity to call
.Fn cap_enter .

This implies a little more heavily that libusb is opening persistent resources.

lib/libusb/libusb.h
281–282

I'd maybe name it a more generic LIBUSB_OPTIOIN_SANDBOX; the libusb that everyone else uses could have platform-specific bits that are required to enable sandboxing with it.

kevans added a comment.Sun, Jul 12, 3:16 PM
In D51865#1334517, @adrian wrote:

hi!

So why are a whole lot of functions getting an fd added to them?

Do the FDs change during the runtime of the tool? Or are they something that could be set
once during some init phase (maybe in a new function which would call cap_enter, rather than
the owner having to call it) and then pulled out of some the libusb backend state?

IMO we really shouldn't have libraries entering capablity mode- libusb may be just one of many, and setting a precedence like that where some will insist on entering the sandbox is bound to end up with some really annoying circumstances.

aokblast added a comment.Mon, Jul 13, 10:13 AM
In D51865#1334517, @adrian wrote:

hi!

So why are a whole lot of functions getting an fd added to them?

As we need to provide the require file descriptor that need to be opened before cap_enter.

Do the FDs change during the runtime of the tool? Or are they something that could be set
once during some init phase (maybe in a new function which would call cap_enter, rather than
the owner having to call it) and then pulled out of some the libusb backend state?

Yes, cap_enter is called in bhyve. The internal fd state is opaque to bhyve.

aokblast added a comment.Mon, Jul 13, 10:15 AM
In D51865#1334521, @kevans wrote:
In D51865#1334517, @adrian wrote:

hi!

So why are a whole lot of functions getting an fd added to them?

Do the FDs change during the runtime of the tool? Or are they something that could be set
once during some init phase (maybe in a new function which would call cap_enter, rather than
the owner having to call it) and then pulled out of some the libusb backend state?

IMO we really shouldn't have libraries entering capablity mode- libusb may be just one of many, and setting a precedence like that where some will insist on entering the sandbox is bound to end up with some really annoying circumstances.

I can somehow agree with you. But how can we integrate to bhyve without this as bhyve is capsicumized by default? We can turn off temporary but it adds complexity on bhyve.

kevans added a comment.Mon, Jul 13, 1:08 PM
In D51865#1334998, @aokblast wrote:
In D51865#1334521, @kevans wrote:
In D51865#1334517, @adrian wrote:

hi!

So why are a whole lot of functions getting an fd added to them?

Do the FDs change during the runtime of the tool? Or are they something that could be set
once during some init phase (maybe in a new function which would call cap_enter, rather than
the owner having to call it) and then pulled out of some the libusb backend state?

IMO we really shouldn't have libraries entering capablity mode- libusb may be just one of many, and setting a precedence like that where some will insist on entering the sandbox is bound to end up with some really annoying circumstances.

I can somehow agree with you. But how can we integrate to bhyve without this as bhyve is capsicumized by default? We can turn off temporary but it adds complexity on bhyve.

No, your approach is fine -- part of his suggestion was that libusb provides something that does these things and calls cap_enter itself as I read it, and that's something that I think should stay outside of libusb.

markj added inline comments.Mon, Jul 13, 1:41 PM
lib/libusb/libusb10.c
2060

Why do we need the caller to explicitly ask the library to be capsicum-safe? Can't it just always use capsicum-safe operations? During library initialization, have libusb open whichever /dev nodes it needs, and keep the descriptors saved in some context structure. Applications which use libusb shouldn't need to change.

2100

Suppose these fields are already set, i.e., LIBUSB_OPTION_CAPSICUMIZE was configured once already. Then the second time, the descriptors will be leaked.

tools/tools/usbtest/usbtest.c
844

Same comment as below about cleaning up.

848

The return value should be checked, see my comment below.

usr.sbin/usbconfig/usbconfig.c
650

Shouldn't the library tell you which rights are needed? The application doesn't really know, it's just treating the usb descriptors as an opaque handle.

659

This is main(), so when you return, the program exits. So, I don't think it's necessary to have all of this cleanup code to close file descriptors.

663
  • This should check for errors (the main one being that the kernel can be compiled without capsicum support).
  • Better to use caph_enter().
939

Revision Contents
Changeset List

PathSize
lib/
libusb/
3 lines
4 lines
6 lines
6 lines
70 lines
10 lines
35 lines
26 lines
66 lines
18 lines
93 lines
tools/
tools/
usbtest/
16 lines
2 lines
9 lines
3 lines
45 lines
usr.sbin/
usbconfig/
21 lines
55 lines

Diff 160244

View Options

lib/libusb/libusb.h

Loading...
View Options

lib/libusb/libusb.3

Loading...
View Options

lib/libusb/libusb01.c

Loading...
View Options

lib/libusb/libusb10.h

Loading...
View Options

lib/libusb/libusb10.c

Loading...
View Options

lib/libusb/libusb10_desc.c

Loading...
View Options

lib/libusb/libusb20.h

Loading...
View Options

lib/libusb/libusb20.3

Loading...
View Options

lib/libusb/libusb20.c

Loading...
View Options

lib/libusb/libusb20_int.h

Loading...
View Options

lib/libusb/libusb20_ugen20.c

Loading...
View Options

tools/tools/usbtest/usb_control_ep_test.c

Loading...
View Options

tools/tools/usbtest/usb_modem_test.c

Loading...
View Options

tools/tools/usbtest/usb_msc_test.c

Loading...
View Options

tools/tools/usbtest/usbtest.h

Loading...
View Options

tools/tools/usbtest/usbtest.c

Loading...
View Options

usr.sbin/usbconfig/dump.c

Loading...
View Options

usr.sbin/usbconfig/usbconfig.c

Loading...