loader: fix incorrect alignment for ELF notes

Fix incorrect alignment for ELF notes, starting address of
name field and descriptor field have a 4-byte alignment.

See refer from:
 - 'Note Section' of 'Executable and Linking Format (ELF)
   Specification' v1.2.
 - Linux implementations,
   https://elixir.bootlin.com/linux/v6.1/source/include/linux/elfnote.h#L56

Fixes: #71

Reported-by: Iulian Barbu <iul@amazon.com>
Signed-off-by: Yong He <alexyonghe@tencent.com>
Co-authored-by: Rob Bradford <robert.bradford@intel.com>
Signed-off-by: Yong He <alexyonghe@tencent.com>

Author: 2 people

CHANGELOG.md

@@ -1,3 +1,10 @@
+# [Upcoming Release]
+
+## Fixed
+- [[#71]](https://github.com/rust-vmm/linux-loader/issues/71) Fix incorrect
+  alignment for ELF notes, starting address of name field and descriptor
+  field have a 4-byte alignment.
+
 # [v0.8.1]
 
 ## Fixed

docs/TESTING.md

@@ -62,7 +62,8 @@ the binaries used in the unit tests.
 | invalid_pvh_note_writer.cpp | test_invalid_pvh_note.bin |
 | dummy_note.cpp | test_dummy_note.bin |
 | basic_elf.cpp | test_elf.bin |
-| pvh_note.cpp | test_elfnote.bin |
+| ignored_phv.cpp | test_elfnote.bin |
+| ignored_phv_8byte_align.cpp | test_elfnote_8byte_align.bin |
 
 #### Example for generating `test_bad_align.bin`
 

docs/elfio_files/ignored_phv_8byte_align.cpp

@@ -0,0 +1,102 @@
+/*
+Copyright (C) 2001-present by Serge Lamikhov-Center
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in
+all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+THE SOFTWARE.
+*/
+
+#include <elfio/elfio.hpp>
+
+using namespace ELFIO;
+
+int main( void )
+{
+    elfio writer;
+
+    // You can't proceed without this function call!
+    writer.create( ELFCLASS64, ELFDATA2LSB );
+
+    writer.set_os_abi( ELFOSABI_LINUX );
+    writer.set_type( ET_EXEC );
+    writer.set_machine( EM_X86_64 );
+
+    // Create a loadable segment
+    segment* load_seg = writer.segments.add();
+    load_seg->set_type( PT_LOAD );
+    load_seg->set_virtual_address( 0x400000 );
+    load_seg->set_physical_address( 0x400000 );
+    load_seg->set_flags( PF_R );
+    load_seg->set_align( 0x200000 );
+
+    // Create a note segment
+    segment* note_seg = writer.segments.add();
+    note_seg->set_type( PT_NOTE );
+    note_seg->set_virtual_address( 0x4000b0 );
+    note_seg->set_physical_address( 0x4000b0 );
+    note_seg->set_flags( PF_R );
+    note_seg->set_align( 0x8 );
+
+    // Create a .note.dummy section, and add it to the note segment.
+    section* dummy_note_sec = writer.sections.add( ".note.dummy" );
+    dummy_note_sec->set_type( SHT_NOTE );
+    dummy_note_sec->set_addr_align( 0x4 );
+    dummy_note_sec->set_flags( SHF_ALLOC );
+    note_section_accessor dummy_note_writer( writer, dummy_note_sec );
+
+    unsigned char dummy_desc[8] = { 0xfe, 0xca, 0xfe, 0xca, 0x00, 0x00 };
+    dummy_note_writer.add_note( 0x01, "dummy", dummy_desc, sizeof( dummy_desc ) );
+
+    note_seg->add_section_index( dummy_note_sec->get_index(),
+                                 dummy_note_sec->get_addr_align() );
+
+    // Create a .note.Xen section, and add it to the note segment.
+    section* xen_note_sec = writer.sections.add( ".note.Xen" );
+    xen_note_sec->set_type( SHT_NOTE );
+    xen_note_sec->set_addr_align( 0x4 );
+    xen_note_sec->set_flags( SHF_ALLOC );
+    note_section_accessor xen_note_writer( writer, xen_note_sec );
+
+    unsigned char xen_descr[8] = { 0x1f, 0xfe, 0xe1, 0x01 };
+    xen_note_writer.add_note( 0x12, "Xen", xen_descr, sizeof( xen_descr ) );
+
+    note_seg->add_section_index( xen_note_sec->get_index(),
+                                 xen_note_sec->get_addr_align() );
+
+    // Create a .note.gnu.build-id section, and add it to the note segment.
+    section* gnu_note_sec = writer.sections.add( ".note.gnu.build-id" );
+    gnu_note_sec->set_type( SHT_NOTE );
+    gnu_note_sec->set_addr_align( 0x4 );
+    gnu_note_sec->set_flags( SHF_ALLOC );
+    note_section_accessor gnu_note_writer( writer, gnu_note_sec );
+
+    unsigned char gnu_descr[20] = { 0x28, 0xcc, 0x3d, 0x3d, 0x89, 0xe5, 0xbf,
+                                    0xc6, 0x07, 0xa8, 0xce, 0xe3, 0x29, 0xcc,
+                                    0x70, 0xd0, 0xbf, 0x34, 0x69, 0x2b };
+    gnu_note_writer.add_note( 0x03, "GNU", gnu_descr, sizeof( gnu_descr ) );
+
+    note_seg->add_section_index( gnu_note_sec->get_index(),
+                                 gnu_note_sec->get_addr_align() );
+    // Setup entry point. Usually, a linker sets this address on base of
+    // ‘_start’ label.
+    writer.set_entry( 0x400108 );
+
+    // Create ELF file
+    writer.save( "test_elfnote_8byte_align.bin" );
+
+    return 0;
+}

src/loader/x86_64/elf/mod.rs

@@ -322,6 +322,14 @@ where
     // the PVH boot protocol.
     const XEN_ELFNOTE_PHYS32_ENTRY: u32 = 18;
 
+    // Alignment of ELF notes, starting address of name field and descriptor field have a 4-byte
+    // alignment.
+    //
+    // See refer from:
+    //  - 'Note Section' of 'Executable and Linking Format (ELF) Specification' v1.2.
+    //  - Linux implementations, https://elixir.bootlin.com/linux/v6.1/source/include/linux/elfnote.h#L56
+    const ELFNOTE_ALIGN: u64 = 4;
+
     // Seek to the beginning of the note segment.
     kernel_image
         .seek(SeekFrom::Start(phdr.p_offset))
@@ -350,8 +358,8 @@ where
         }
 
         // Skip the note header plus the size of its fields (with alignment).
-        let namesz_aligned = align_up(u64::from(nhdr.n_namesz), phdr.p_align)?;
-        let descsz_aligned = align_up(u64::from(nhdr.n_descsz), phdr.p_align)?;
+        let namesz_aligned = align_up(u64::from(nhdr.n_namesz), ELFNOTE_ALIGN)?;
+        let descsz_aligned = align_up(u64::from(nhdr.n_descsz), ELFNOTE_ALIGN)?;
 
         // `namesz` and `descsz` are both `u32`s. We need to also verify for overflow, to be sure
         // we do not lose information.
@@ -385,7 +393,7 @@ where
     kernel_image
         .seek(SeekFrom::Current(
             // Safe conversion since it is not losing data.
-            align_up(u64::from(nhdr.n_namesz), phdr.p_align)? as i64 - PVH_NOTE_STR_SZ as i64,
+            align_up(u64::from(nhdr.n_namesz), ELFNOTE_ALIGN)? as i64 - PVH_NOTE_STR_SZ as i64,
         ))
         .map_err(|_| Error::SeekNoteHeader)?;
 
@@ -449,6 +457,10 @@ mod tests {
         include_bytes!("test_elfnote.bin").to_vec()
     }
 
+    fn make_elfnote_8byte_align() -> Vec<u8> {
+        include_bytes!("test_elfnote_8byte_align.bin").to_vec()
+    }
+
     fn make_dummy_elfnote() -> Vec<u8> {
         include_bytes!("test_dummy_note.bin").to_vec()
     }
@@ -457,7 +469,7 @@ mod tests {
         include_bytes!("test_invalid_pvh_note.bin").to_vec()
     }
 
-    fn make_bad_align() -> Vec<u8> {
+    fn make_elfnote_bad_align() -> Vec<u8> {
         include_bytes!("test_bad_align.bin").to_vec()
     }
 
@@ -585,13 +597,31 @@ mod tests {
     }
 
     #[test]
-    fn test_bad_align() {
-        let gm = GuestMemoryMmap::from_ranges(&[(GuestAddress(0x0), (0x1000_0000_usize))]).unwrap();
-        let bad_align_image = make_bad_align();
-        assert_eq!(
-            Some(KernelLoaderError::Elf(Error::Align)),
-            Elf::load(&gm, None, &mut Cursor::new(&bad_align_image), None).err()
-        );
+    fn test_load_pvh_with_align() {
+        // Alignment of ELF notes is always const value (4-bytes), ELF notes parse should not get Align
+        // error.
+        {
+            let gm =
+                GuestMemoryMmap::from_ranges(&[(GuestAddress(0x0), (0x1000_0000_usize))]).unwrap();
+            let bad_align_image = make_elfnote_bad_align();
+            assert_ne!(
+                Some(KernelLoaderError::Elf(Error::Align)),
+                Elf::load(&gm, None, &mut Cursor::new(&bad_align_image), None).err()
+            );
+        }
+
+        // Alignment of ELF notes is always const value (4-byte), ELF notes parse should always
+        // success even there is incorrect p_align in phdr.
+        {
+            let gm = create_guest_mem();
+            let pvhnote_image = make_elfnote_8byte_align();
+            let loader_result =
+                Elf::load(&gm, None, &mut Cursor::new(&pvhnote_image), None).unwrap();
+            assert_eq!(
+                loader_result.pvh_boot_cap,
+                PvhBootCapability::PvhEntryPresent(GuestAddress(0x1e1fe1f))
+            );
+        }
     }
 
     #[test]