Page MenuHomeFreeBSD
Differential D51865

libusb: capsicumize libusb
AcceptedPublic
Actions

Authored by aokblast on Aug 11 2025, 3:54 PM.
Tags
None
Referenced Files
F166432662: D51865.diff
Thu, Aug 13, 1:29 PM
F166343805: D51865.id160587.diff
Thu, Aug 13, 12:51 AM
F166339407: D51865.id160193.diff
Thu, Aug 13, 12:20 AM
F166337004: D51865.id.diff
Thu, Aug 13, 12:02 AM
Unknown Object (File)
Wed, Aug 12, 2:16 PM
Unknown Object (File)
Wed, Aug 12, 1:33 PM
Unknown Object (File)
Wed, Aug 12, 1:17 AM
Unknown Object (File)
Wed, Aug 12, 12:14 AM
View All Files
Subscribers
adrian
emaste
imp
jonathan
kevans
kib
ziaee

Details

Reviewers
jfree
pjd
markj
oshogbo
lwhsu
adrian
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 66193
Build 63076: 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
641

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
641

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–905

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.

641

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.Jul 12 2026, 2:48 PM
adrian added a subscriber: adrian.Jul 12 2026, 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.Jul 12 2026, 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.Jul 12 2026, 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.Jul 13 2026, 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.Jul 13 2026, 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.Jul 13 2026, 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.Jul 13 2026, 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
647

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.

656

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.

660
  • This should check for errors (the main one being that the kernel can be compiled without capsicum support).
  • Better to use caph_enter().
936
aokblast updated this revision to Diff 182658.Sat, Jul 25, 2:33 AM

Reflect @markj's comment

Harbormaster completed remote builds in B75113: Diff 182658.Sat, Jul 25, 2:33 AM
adrian accepted this revision.Sat, Jul 25, 2:38 AM

much better, yay! thanks!

This revision is now accepted and ready to land.Sat, Jul 25, 2:38 AM
markj added inline comments.Mon, Jul 27, 3:03 PM
lib/libusb/libusb20_ugen20.c
93

I think you need capiscum_helpers.h for this.

usr.sbin/usbconfig/usbconfig.c
574

dump_device_info()->_device_desc()->load_vendors() will try to open _PATH_LUSBVDB, and that'll fail in capability mode.

I think this is straightforward to deal with, just make sure usb_vendors is initialized before you enter capability mode.

markj added inline comments.Mon, Jul 27, 3:07 PM
lib/libusb/libusb20.h
306–307

There is some code under share/examples/libusb20 which uses these interfaces, it should be updated too.

Why is it okay to change the library ABI?

emaste added a subscriber: emaste.Mon, Jul 27, 5:19 PM

We'll need a so bump for libusb (see D55687, D55686) so we should coordinate to have all of these changes happen together.

aokblast added inline comments.Mon, Aug 10, 3:30 PM
lib/libusb/libusb20.h
306–307

Yes, we definately needs to merge ABI tagging back to fix it

aokblast updated this revision to Diff 183962.Thu, Aug 13, 5:36 AM

Versioning

This revision now requires review to proceed.Thu, Aug 13, 5:36 AM
Harbormaster completed remote builds in B75546: Diff 183962.Thu, Aug 13, 5:36 AM
markj accepted this revision.Thu, Aug 13, 2:34 PM
markj added a subscriber: kib.

Mostly looks good to me.

lib/libusb/Symbol.map
258 ↗(On Diff #183962)

This is off by one. There is no FBSD_1.10 (yet), FBSD_1.9 is for symbols added in FreeBSD 16.0. See lib/libc/Versions.def.

The existing symbols should be in FBSD_1.8, not FBSD_1.9. It doesn't really matter in practice, I think, but I am not too knowledgeable on this topic. Maybe @kib can suggest whether it is worth fixing this.

share/examples/libusb20/bulk.c
229 ↗(On Diff #183962)

I am not sure it makes sense to enter capability mode in this example, but ok.

usr.sbin/usbconfig/dump.c
367

BTW, it is weird to malloc() this structure. It's just a STAILQ head, it can be defined globally with

static struct usb_vendors usb_vendors = STAILQ_INITIALIZER(&usb_vendors);

Then, just test STAILQ_EMPTY(&usb_vendors) to see if we need to read from the file.

469

Why does this need to be called? You are already calling it in main().

This revision is now accepted and ready to land.Thu, Aug 13, 2:34 PM

Revision Contents
Changeset List

PathSize
lib/
libusb/
3 lines
7 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/
3 lines
18 lines
52 lines

Diff 160245

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.h

Loading...
View Options

usr.sbin/usbconfig/dump.c

Loading...
View Options

usr.sbin/usbconfig/usbconfig.c

Loading...