Page MenuHomeFreeBSD

bhyve: error out if fwcfg user file isn't read completely
ClosedPublic

Authored by corvink on May 12 2023, 5:40 AM.
Tags
None
Referenced Files
F102551462: D40076.id.diff
Wed, Nov 13, 11:02 PM
Unknown Object (File)
Oct 10 2024, 10:28 AM
Unknown Object (File)
Oct 7 2024, 12:38 AM
Unknown Object (File)
Sep 23 2024, 1:12 PM
Unknown Object (File)
Sep 18 2024, 9:39 PM
Unknown Object (File)
Sep 17 2024, 11:28 PM
Unknown Object (File)
Sep 16 2024, 11:50 AM
Unknown Object (File)
Sep 4 2024, 9:17 AM

Details

Summary
At the moment, fwcfg reads the file once at startup and passes these
data to the guest. Therefore, we should always read the whole file.
Otherwise we should error out.

Additionally, GCC12 complains that the comparison whether
fwcfg_file->size is lower than 0 is always false due to the limited
range of data type.

Diff Detail

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

Event Timeline

rew added inline comments.
usr.sbin/bhyve/qemu_fwcfg.c
603–606

Why is a partial read of the file not an error?

usr.sbin/bhyve/qemu_fwcfg.c
603–606

to expand the previous question..

should we ensure that all bytes are read or error out?

and why not use the same pattern that you used in pci_passthru.c using mmap()?

  • error out on partial reads
corvink retitled this revision from bhyve: fix comparison when reading fwcfg user files to bhyve: error out if fwcfg user file isn't read completely.May 12 2023, 6:27 AM
corvink edited the summary of this revision. (Show Details)
corvink added inline comments.
usr.sbin/bhyve/qemu_fwcfg.c
603–606

You're right. Partial reads don't make sense.

What's the advantage of using mmap + memcpy instead of just read?

usr.sbin/bhyve/qemu_fwcfg.c
596–597

I don't think this is correct since a signed type is being returned into an unsigned variable.

granted - if there is a read error, the check below will still do the right thing unless the file size happens to be 4G.

603–606

What's the advantage of using mmap + memcpy instead of just read?

I'm not aware of an advantage in using one or over the other in this scenario.

corvink marked an inline comment as done.
  • save read output in a signed variable
This revision is now accepted and ready to land.May 15 2023, 2:26 PM