[Task] Review FV `new_from_address()` safety invariants

[makubacki]

VolumeRef::new_from_address() and FirmwareVolume::new_from_address() are unsafe fn functions whose safety contracts are difficult for callers to satisfy correctly.

Callers must guarantee the backing memory is valid for fv_length bytes, but fv_length is not known until the function reads it from untrusted memory at base_address. The API only accepts an address and not an independently trusted size so callers have no way to supply a bound even when they have one. As a result, the function passes an untrusted fv_length to slice::from_raw_parts(), and the internal validation in new() is defeated because the buffer was created from the same value being checked.

patina/sdk/patina_ffs/src/volume.rs

Lines 203 to 244 in b059896

	/// Instantiate a new FirmwareVolume from a base address.
	///
	/// ## Safety
	///
	/// Caller must ensure that base_address is the address of the start of a firmware volume.
	/// Caller must ensure that the lifetime of the buffer at base_address is longer than the
	/// returned VolumeRef.
	///
	/// ## Examples
	///
	/// ```rust no_run
	/// use patina_ffs::volume::{Volume, VolumeRef};
	/// use patina::pi::fw_fs::fv::BlockMapEntry;
	///
	/// let fv_bytes = Volume::new(vec![BlockMapEntry { num_blocks: 1, length: 4096 }])
	/// .serialize()
	/// .unwrap();
	/// let base = fv_bytes.as_ptr() as u64;
	/// let fv_ref = unsafe { VolumeRef::new_from_address(base) }.unwrap();
	/// assert!(fv_ref.size() >= 4096);
	/// ```
	pub unsafe fn new_from_address(base_address: u64) -> Result<Self, FirmwareFileSystemError> {
	if base_address == ptr::null::<fv::Header>() as u64 {
	return Err(FirmwareFileSystemError::InvalidParameter);
	}
	// SAFETY: caller must ensure that the base_address is safe to read for enough bytes to read
	// the fv_header structure.
	let fv_header = unsafe { ptr::read_unaligned(base_address as *const fv::Header) };
	if fv_header.signature != u32::from_le_bytes(*b"_FVH") {
	// base_address is not the start of a firmware volume.
	return Err(FirmwareFileSystemError::DataCorrupt);
	}
	// SAFETY: caller must ensure that the base_address is valid and points
	// to memory that contains a valid firmware volume as part of the safety
	// contract for this unsafe function. The basic signature check provides
	// some protection, but if that passes and the header data is invalid
	// (particularly the length) then this could result in memory safety
	// violations.
	let fv_buffer = unsafe { slice::from_raw_parts(base_address as *const u8, fv_header.fv_length as usize) };
	Self::new(fv_buffer)
	}

---

As unsafe fn functions, they require that the caller must ensure this address is valid for N bytes, but the contract only works if the caller can actually know N. In this instance, the new_from_address function takes base_address then reads fv_length from the memory at that address and uses it immediately. Whether the contract can lead to real Undefined Behavior depends on whether callers can independently bound the memory region. Some callers do this, for example Firmware Volume HOBS have both base_address and length fields.

---

This issue tracks reviewing the safety conditions and interface/documentation to determine if there is a clearer and safer way to have the size be passed.