Page MenuHomeFreeBSD

bhyve/virtio-scsi: Don't invoke iov_to_buf() in an assert() expression
ClosedPublic

Authored by rosenfeld_grumpf.hope-2000.org on Mar 10 2026, 7:18 PM.
Tags
None
Referenced Files
F162635971: D55803.diff
Wed, Jul 15, 7:12 AM
F162635970: D55803.diff
Wed, Jul 15, 7:12 AM
F162608244: D55803.id177321.diff
Wed, Jul 15, 12:21 AM
Unknown Object (File)
Tue, Jul 14, 7:34 AM
Unknown Object (File)
Wed, Jul 8, 11:53 AM
Unknown Object (File)
Wed, Jul 8, 7:09 AM
Unknown Object (File)
Mon, Jul 6, 5:13 AM
Unknown Object (File)
Sun, Jul 5, 3:15 PM

Details

Summary

If anyone would build bhyve with -DNDEBUG, any code in the expression
in assert() won't be executed. Remove the assertion and check the
returned size from iov_to_buf() explicitly, exiting if it is wrong.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 72040
Build 68923: arc lint + arc unit

Event Timeline

usr.sbin/bhyve/pci_virtio_scsi.c
753
des requested changes to this revision.Mar 11 2026, 12:28 PM
des added a subscriber: des.
des added inline comments.
usr.sbin/bhyve/pci_virtio_scsi.c
751

This still fails to build with gcc.

This revision now requires changes to proceed.Mar 11 2026, 12:28 PM
usr.sbin/bhyve/pci_virtio_scsi.c
751

That's not surprising, given this is a fix for a different issue. Please see https://reviews.freebsd.org/D55800 for the proper fix of the build failure with gcc.

This revision now requires review to proceed.Mar 11 2026, 12:53 PM

Or, something along the lines of:

size_t res __maybe_unused;
res = iov_to_buf(...);
assert (res == VTSCSI_IN_HEADER_LEN(q->vsq_sc);

Or, something along the lines of:

size_t res __maybe_unused;
res = iov_to_buf(...);
assert (res == VTSCSI_IN_HEADER_LEN(q->vsq_sc);

There doesn't seem to be a general definition of __maybe_unused in FreeBSD, or I'm just too dense to find it. Some externals sources like ZFS seem to have it, though.

Perhaps I could use maybe_unused, but that's a C23 feature.

The commit should include a Fixes: 2a514d377b37 ("bhyve/virtio-scsi: Preallocate all I/O requests") tag.

This revision is now accepted and ready to land.Apr 15 2026, 1:20 PM

There doesn't seem to be a general definition of __maybe_unused in FreeBSD

Ah, yes. We seem two have three copies of the #define in our tree, two under sys/, but it's not generally available. We could perhaps define it in cdefs.h, but independent of this change.

There doesn't seem to be a general definition of __maybe_unused in FreeBSD

Ah, yes. We seem two have three copies of the #define in our tree, two under sys/, but it's not generally available. We could perhaps define it in cdefs.h, but independent of this change.

Done, please see https://reviews.freebsd.org/D56517

This revision now requires review to proceed.Apr 20 2026, 9:04 AM

Updated commit message for return to using assert

This revision was not accepted when it landed; it landed in state Needs Review.May 6 2026, 5:39 PM
This revision was automatically updated to reflect the committed changes.