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
F158677856: D55803.id175085.diff
Thu, Jun 4, 2:50 PM
Unknown Object (File)
Sun, May 24, 7:41 PM
Unknown Object (File)
Sun, May 24, 7:31 PM
Unknown Object (File)
Fri, May 22, 7:11 AM
Unknown Object (File)
Tue, May 19, 9:11 PM
Unknown Object (File)
Mon, May 18, 11:26 AM
Unknown Object (File)
Mon, May 18, 11:21 AM
Unknown Object (File)
Mon, May 18, 8:28 AM

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 71323
Build 68206: 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.Wed, May 6, 5:39 PM
This revision was automatically updated to reflect the committed changes.