writer: check conversions from usize to u32 #10
Conversation
danielverkamp
force-pushed
the
unchecked-conversions
branch
from
302517f
to
d22c8ed
Compare
| @@ -107,7 +107,7 @@ impl FdtWriter { | |||
| /// # Arguments | |||
| /// | |||
| /// `mem_reservations` - reserved physical memory regions to list in the FDT header. | |||
| pub fn new(mem_reservations: &[FdtReserveEntry]) -> Self { | |||
| pub fn new(mem_reservations: &[FdtReserveEntry]) -> Result<Self> { | |||
There was a problem hiding this comment.
Can we add a regression test for this? It is taking some time to initialize a large array, but if that's a problem we can have it as #[ignore] and only run it in the CI (we would need to update the Buildkite pipeline for that in rust-vmm-ci if we go down that path).
There was a problem hiding this comment.
I added a test based on the one in #6 and it seems to work (albeit very slowly). My main concern was that it will allocate 8 GiB of RAM at least (the original 4G array of FdtReserveEntry structures and a copy in the FdtWriter data), which could be a problem for smaller machines or tests running in a restricted envinronment; I guess we will see if this works on the CI system.
These conversions were previously unchecked casts, which would potentially truncate large values. In particular, large reservemap arrays would cause overflows when calculating the size of the DT struct in the finish function. Fixes #7. Signed-off-by: Daniel Verkamp <dverkamp@chromium.org>
danielverkamp
force-pushed
the
unchecked-conversions
branch
from
d22c8ed
to
fd09484
Compare
|
|
||
| #[test] | ||
| fn test_overflow_subtract() { | ||
| let overflow_size = std::u32::MAX / size_of::<FdtReserveEntry>() as u32 - 3; |
There was a problem hiding this comment.
nit: std::u32::MAX will be deprecated. We can use u32::MAX instead.
These conversions were previously unchecked casts, which would
potentially truncate large values. In particular, large reservemap
arrays would cause overflows when calculating the size of the DT struct
in the finish function.
Fixes #6.
Signed-off-by: Daniel Verkamp dverkamp@chromium.org