Skip to content

Commit 0764ad3

Browse files
Alexandra IordacheSamuel Ortiz
authored andcommitted
fix panic condition in align_up function
Fixed a panic condition that could cause loading malformed ELF kernel images containing unaligned values in the `n_namesz` and `n_descsz` fields of ELFNOTE headers to lead to the abrupt termination of the vmm process. Signed-off-by: Alexandra Iordache <aghecen@amazon.com> Signed-off-by: Andreea Florescu <fandree@amazon.com>
1 parent 4e6d613 commit 0764ad3

File tree

2 files changed

+32
-12
lines changed

2 files changed

+32
-12
lines changed

src/loader/x86_64/elf/mod.rs

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use std::fmt;
1616
use std::io::{Read, Seek, SeekFrom};
1717
use std::mem;
18+
use std::result;
1819

1920
use vm_memory::{Address, ByteValued, Bytes, GuestAddress, GuestMemory, GuestUsize};
2021

@@ -29,9 +30,11 @@ unsafe impl ByteValued for elf::Elf64_Phdr {}
2930
#[derive(Debug, PartialEq)]
3031
/// Elf kernel loader errors.
3132
pub enum Error {
33+
/// Invalid alignment.
34+
Align,
3235
/// Loaded big endian binary on a little endian platform.
3336
BigEndianElfOnLittle,
34-
/// Invalid ELF magic number
37+
/// Invalid ELF magic number.
3538
InvalidElfMagicNumber,
3639
/// Invalid program header size.
3740
InvalidProgramHeaderSize,
@@ -64,6 +67,7 @@ pub enum Error {
6467
impl fmt::Display for Error {
6568
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
6669
let desc = match self {
70+
Error::Align => "Invalid alignment",
6771
Error::BigEndianElfOnLittle => {
6872
"Trying to load big-endian binary on little-endian machine"
6973
}
@@ -332,8 +336,8 @@ where
332336

333337
// Skip the note header plus the size of its fields (with alignment).
334338
read_size += nhdr_sz
335-
+ align_up(u64::from(nhdr.n_namesz), n_align)
336-
+ align_up(u64::from(nhdr.n_descsz), n_align);
339+
+ align_up(u64::from(nhdr.n_namesz), n_align)?
340+
+ align_up(u64::from(nhdr.n_descsz), n_align)?;
337341

338342
kernel_image
339343
.seek(SeekFrom::Start(phdr.p_offset + read_size as u64))
@@ -350,7 +354,7 @@ where
350354
// just skip the name field contents.
351355
kernel_image
352356
.seek(SeekFrom::Current(
353-
align_up(u64::from(nhdr.n_namesz), n_align) as i64 - PVH_NOTE_STR_SZ as i64,
357+
align_up(u64::from(nhdr.n_namesz), n_align)? as i64 - PVH_NOTE_STR_SZ as i64,
354358
))
355359
.map_err(|_| Error::SeekNoteHeader)?;
356360

@@ -372,18 +376,20 @@ where
372376
)))
373377
}
374378

375-
/// Align address upwards. Taken from x86_64 crate:
376-
/// https://docs.rs/x86_64/latest/x86_64/fn.align_up.html
379+
/// Align address upwards. Adapted from x86_64 crate:
380+
/// https://docs.rs/x86_64/latest/x86_64/addr/fn.align_up.html
377381
///
378-
/// Returns the smallest x with alignment `align` so that x >= addr. The alignment must be
379-
/// a power of 2.
380-
fn align_up(addr: u64, align: u64) -> usize {
381-
assert!(align.is_power_of_two(), "`align` must be a power of two");
382+
/// Returns the smallest x with alignment `align` so that x >= addr if the alignment is a power of
383+
/// 2, or an error otherwise.
384+
fn align_up(addr: u64, align: u64) -> result::Result<usize, Error> {
385+
if !align.is_power_of_two() {
386+
return Err(Error::Align);
387+
}
382388
let align_mask = align - 1;
383389
if addr & align_mask == 0 {
384-
addr as usize // already aligned
390+
Ok(addr as usize) // already aligned
385391
} else {
386-
((addr | align_mask) + 1) as usize
392+
Ok(((addr | align_mask) + 1) as usize)
387393
}
388394
}
389395

@@ -417,6 +423,10 @@ mod tests {
417423
include_bytes!("test_badnote.bin").to_vec()
418424
}
419425

426+
fn make_bad_align() -> Vec<u8> {
427+
include_bytes!("test_align.bin").to_vec()
428+
}
429+
420430
#[test]
421431
fn test_load_elf() {
422432
let gm = create_guest_mem();
@@ -539,4 +549,14 @@ mod tests {
539549
Elf::load(&gm, None, &mut Cursor::new(&badnote_image), None).err()
540550
);
541551
}
552+
553+
#[test]
554+
fn test_bad_align() {
555+
let gm = GuestMemoryMmap::from_ranges(&[(GuestAddress(0x0), (0x10_000_000usize))]).unwrap();
556+
let bad_align_image = make_bad_align();
557+
assert_eq!(
558+
Some(KernelLoaderError::Elf(Error::Align)),
559+
Elf::load(&gm, None, &mut Cursor::new(&bad_align_image), None).err()
560+
);
561+
}
542562
}

src/loader/x86_64/elf/test_align.bin

644 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)