Skip to content

Various Fixes of Potential Undefined Behavior (LLM Assisted) (4/N) - #2080

Merged
phip1611 merged 4 commits into
mainfrom
ub-fixes-4
Sep 14, 2026
Merged

phip1611 merged 4 commits into
mainfrom
ub-fixes-4

Conversation

@phip1611

Copy link
Copy Markdown
Member

Follow-up of #2079.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@phip1611 phip1611 changed the title Various Fixes of Potential Undefined Behavior (LLM Assisted) (4/N)- #2079 Various Fixes of Potential Undefined Behavior (LLM Assisted) (4/N) Sep 14, 2026
`Handle` wraps a `NonNull`, and `ProtocolsPerHandle` hands out `&Guid`
references, but the pointer arrays the firmware fills were exposed
without checking their entries. A null entry therefore produced an
invalid `Handle` or a null reference, which is undefined behavior even
before the value is used.

Validate the entries in `locate_handle`, `locate_handle_buffer` and
`protocols_per_handle`, and report `INVALID_PARAMETER` if the firmware
returned a null one. `find_handles` is covered through `locate_handle`.

`locate_device_path` passed the pointer the firmware writes back
straight to `DevicePath::from_ffi_ptr`, which dereferences it. Check it
for null as well.

Found with Miri using a mocked firmware.
`AtaResponse::read_buffer`, `NvmeResponse::{transfer_buffer,
metadata_buffer}` and `ScsiResponse::{read_buffer, sense_data}` built a
slice from the length the firmware writes into the command packet. The
length is an in-out parameter, so nothing stops a controller from
reporting more than the buffer can hold, which yields a slice that
points past the allocation.

Remember the capacity of the buffer that was handed to the firmware when
it is attached to the request, and clamp the reported length to it.

Found with Miri using a mocked firmware.
`HttpHelper::request`, `response_first` and `response_more` poll the
network stack until the token they handed to the firmware completes. A
failing `poll` returned early through `?` while the token was still
pending. The firmware keeps a pointer to that token, and to the message
and buffers it references, all of which live in the stack frame that is
then gone, so the next completion wrote into freed memory. The UEFI
specification explicitly allows `Poll` to fail, for example with
`DEVICE_ERROR` or `TIMEOUT`.

Track the pending token in a guard that cancels it when it is dropped,
so every early exit takes the token back from the firmware.

The response data was additionally passed as a pointer derived from a
mutable reference that was coerced to `*const`, although the driver
fills in the status code. Keep the write permission on that pointer.

Found with Miri using a mocked firmware.
`PointerMode` and `PointerState` duplicated `SimplePointerMode` and
`SimplePointerState` from `uefi-raw`, except that they declared the
`BOOLEAN` fields written by the firmware as Rust `bool`. `Pointer::mode`
reinterpreted the mode structure owned by the firmware as a
`PointerMode`, and `Pointer::read_state` let the firmware write into a
`PointerState`. `BOOLEAN` is a byte that may hold any value, and every
value other than 0 and 1 is an invalid `bool`, which is undefined
behavior as soon as it is produced.

Drop the duplicates and re-export the `uefi-raw` types under the
existing names instead, as `AbsolutePointer` already uses its raw types
directly. Both accessors then hand out exactly what the firmware wrote
and no cast is left, so the invalid value can no longer be constructed.
Callers keep their imports and convert a button with `bool::from`.

Found with Miri using a mocked firmware.
@phip1611
phip1611 enabled auto-merge September 14, 2026 06:39
@phip1611
phip1611 added this pull request to the merge queue Sep 14, 2026
Merged via the queue into main with commit 5506db8 Sep 14, 2026
19 of 20 checks passed
@phip1611
phip1611 deleted the ub-fixes-4 branch September 14, 2026 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant