Kernel: Don't use references or pointers to physical addresses

Now the DMIDecoder code is more safer, because we don't use raw pointers
or references to objects or data that are located in the physical
address space, so an accidental dereference cannon happen easily.
Instead, we use the PhysicalAddress class to represent those addresses.

Also, the initializer_parser() method is simplified.

Kernel/ACPI/DMIDecoder.cpp

@@ -60,59 +60,60 @@ void DMIDecoder::initialize_untrusted()
     }
 }
 
-void DMIDecoder::set_64_bit_entry_initialization_values(SMBIOS::EntryPoint64bit& entry)
+void DMIDecoder::set_64_bit_entry_initialization_values(PhysicalAddress entry)
 {
-    kprintf("DMIDecoder: SMBIOS 64bit Entry point @ P 0x%x\n", m_entry64bit_point);
+    kprintf("DMIDecoder: SMBIOS 64bit Entry point @ P 0x%x\n", m_entry64bit_point.get());
     m_use_64bit_entry = true;
 
-    auto region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((u32)&entry)), PAGE_ROUND_UP(SMBIOS_SEARCH_AREA_SIZE), "DMI Decoder 64 bit Initialization", Region::Access::Read, false, false);
-    auto& entry_ptr = *(SMBIOS::EntryPoint64bit*)region->vaddr().offset(offset_in_page((u32)&entry)).as_ptr();
-    m_structure_table = (SMBIOS::TableHeader*)entry_ptr.table_ptr;
+    auto region = MM.allocate_kernel_region(entry.page_base(), PAGE_ROUND_UP(SMBIOS_SEARCH_AREA_SIZE), "DMI Decoder 64 bit Initialization", Region::Access::Read, false, false);
+    auto& entry_ptr = *(SMBIOS::EntryPoint64bit*)region->vaddr().offset(entry.offset_in_page().get()).as_ptr();
+    m_structure_table = PhysicalAddress(entry_ptr.table_ptr);
     m_structures_count = entry_ptr.table_maximum_size;
     m_table_length = entry_ptr.table_maximum_size;
 }
 
-void DMIDecoder::set_32_bit_entry_initialization_values(SMBIOS::EntryPoint32bit& entry)
+void DMIDecoder::set_32_bit_entry_initialization_values(PhysicalAddress entry)
 {
-    kprintf("DMIDecoder: SMBIOS 32bit Entry point @ P 0x%x\n", m_entry32bit_point);
+    kprintf("DMIDecoder: SMBIOS 32bit Entry point @ P 0x%x\n", m_entry32bit_point.get());
     m_use_64bit_entry = false;
 
-    auto region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((u32)&entry)), PAGE_ROUND_UP(SMBIOS_SEARCH_AREA_SIZE), "DMI Decoder 32 bit Initialization", Region::Access::Read, false, false);
-    auto& entry_ptr = *(SMBIOS::EntryPoint32bit*)region->vaddr().offset(offset_in_page((u32)&entry)).as_ptr();
+    auto region = MM.allocate_kernel_region(entry.page_base(), PAGE_ROUND_UP(SMBIOS_SEARCH_AREA_SIZE), "DMI Decoder 32 bit Initialization", Region::Access::Read, false, false);
+    auto& entry_ptr = *(SMBIOS::EntryPoint32bit*)region->vaddr().offset(entry.offset_in_page().get()).as_ptr();
 
-    m_structure_table = (SMBIOS::TableHeader*)entry_ptr.legacy_structure.smbios_table_ptr;
+    m_structure_table = PhysicalAddress(entry_ptr.legacy_structure.smbios_table_ptr);
     m_structures_count = entry_ptr.legacy_structure.smbios_tables_count;
     m_table_length = entry_ptr.legacy_structure.smboios_table_length;
 }
 
 void DMIDecoder::initialize_parser()
 {
-    if (m_entry32bit_point != nullptr || m_entry64bit_point != nullptr) {
-        m_operable = true;
-        kprintf("DMI Decoder is enabled\n");
-        if (m_entry64bit_point != nullptr) {
-            set_64_bit_entry_initialization_values(*m_entry64bit_point);
-        } else if (m_entry32bit_point != nullptr) {
-            set_32_bit_entry_initialization_values(*m_entry32bit_point);
-        }
-        kprintf("DMIDecoder: Data table @ P 0x%x\n", m_structure_table);
-        enumerate_smbios_tables();
-    } else {
+
+    if (m_entry32bit_point.is_null() && m_entry64bit_point.is_null()) {
         m_operable = false;
         kprintf("DMI Decoder is disabled. Cannot find SMBIOS tables.\n");
+        return;
     }
+
+    m_operable = true;
+    kprintf("DMI Decoder is enabled\n");
+    if (!m_entry64bit_point.is_null()) {
+        set_64_bit_entry_initialization_values(m_entry64bit_point);
+    } else if (!m_entry32bit_point.is_null()) {
+        set_32_bit_entry_initialization_values(m_entry32bit_point);
+    }
+    kprintf("DMIDecoder: Data table @ P 0x%x\n", m_structure_table.get());
+    enumerate_smbios_tables();
 }
 
 void DMIDecoder::enumerate_smbios_tables()
 {
 
     u32 table_length = m_table_length;
-    SMBIOS::TableHeader* p_table_ptr = m_structure_table;
+    auto p_table = m_structure_table;
 
-    PhysicalAddress paddr = PhysicalAddress(page_base_of((uintptr_t)p_table_ptr));
-    auto region = MM.allocate_kernel_region(paddr, PAGE_ROUND_UP(table_length), "DMI Decoder Enumerating SMBIOS", Region::Access::Read, false, false);
+    auto region = MM.allocate_kernel_region(p_table.page_base(), PAGE_ROUND_UP(table_length), "DMI Decoder Enumerating SMBIOS", Region::Access::Read, false, false);
+    volatile SMBIOS::TableHeader* v_table_ptr = (SMBIOS::TableHeader*)region->vaddr().offset(p_table.offset_in_page().get()).as_ptr();
 
-    volatile SMBIOS::TableHeader* v_table_ptr = (SMBIOS::TableHeader*)region->vaddr().offset(offset_in_page((uintptr_t)p_table_ptr)).as_ptr();
 #ifdef SMBIOS_DEBUG
     dbgprintf("DMIDecoder: Total Table length %d\n", m_table_length);
 #endif
@@ -120,7 +121,7 @@ void DMIDecoder::enumerate_smbios_tables()
     u32 structures_count = 0;
     while (table_length > 0) {
 #ifdef SMBIOS_DEBUG
-        dbgprintf("DMIDecoder: Examining table @ P 0x%x V 0x%x\n", p_table_ptr, v_table_ptr);
+        dbgprintf("DMIDecoder: Examining table @ P 0x%x V 0x%x\n", p_table.get(), v_table_ptr);
 #endif
         structures_count++;
         if (v_table_ptr->type == (u8)SMBIOS::TableType::EndOfTable) {
@@ -128,25 +129,23 @@ void DMIDecoder::enumerate_smbios_tables()
             break;
         }
         kprintf("DMIDecoder: Detected table with type %d\n", v_table_ptr->type);
-        m_smbios_tables.append(p_table_ptr);
+        m_smbios_tables.append(p_table);
         table_length -= v_table_ptr->length;
 
-        size_t table_size = get_table_size(*p_table_ptr);
-        p_table_ptr = (SMBIOS::TableHeader*)((uintptr_t)p_table_ptr + table_size);
+        size_t table_size = get_table_size(p_table);
+        p_table = p_table.offset(table_size);
         v_table_ptr = (SMBIOS::TableHeader*)((uintptr_t)v_table_ptr + table_size);
 #ifdef SMBIOS_DEBUG
-        dbgprintf("DMIDecoder: Next table @ P 0x%x\n", p_table_ptr);
+        dbgprintf("DMIDecoder: Next table @ P 0x%x\n", p_table.get());
 #endif
-        if (p_table_ptr == nullptr)
-            break;
     }
     m_structures_count = structures_count;
 }
 
-size_t DMIDecoder::get_table_size(SMBIOS::TableHeader& table)
+size_t DMIDecoder::get_table_size(PhysicalAddress table)
 {
-    auto region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((u32)&table)), PAGE_ROUND_UP(m_table_length), "DMI Decoder Determining table size", Region::Access::Read, false, false);
-    auto& table_v_ptr = (SMBIOS::TableHeader&)*region->vaddr().offset(offset_in_page((u32)&table)).as_ptr();
+    auto region = MM.allocate_kernel_region(table.page_base(), PAGE_ROUND_UP(m_table_length), "DMI Decoder Determining table size", Region::Access::Read, false, false);
+    auto& table_v_ptr = (SMBIOS::TableHeader&)*region->vaddr().offset(table.offset_in_page().get()).as_ptr();
 #ifdef SMBIOS_DEBUG
     dbgprintf("DMIDecoder: table legnth - 0x%x\n", table_v_ptr.length);
 #endif
@@ -164,45 +163,45 @@ size_t DMIDecoder::get_table_size(SMBIOS::TableHeader& table)
     return table_v_ptr.length + index + 1;
 }
 
-SMBIOS::TableHeader* DMIDecoder::get_next_physical_table(SMBIOS::TableHeader& p_table)
+PhysicalAddress DMIDecoder::get_next_physical_table(PhysicalAddress p_table)
 {
-    return (SMBIOS::TableHeader*)((uintptr_t)&p_table + get_table_size(p_table));
+    return p_table.offset(get_table_size(p_table));
 }
 
-SMBIOS::TableHeader* DMIDecoder::get_smbios_physical_table_by_handle(u16 handle)
+PhysicalAddress DMIDecoder::get_smbios_physical_table_by_handle(u16 handle)
 {
 
-    for (auto* table : m_smbios_tables) {
-        if (!table)
+    for (auto table : m_smbios_tables) {
+        if (table.is_null())
             continue;
-        auto region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((uintptr_t)table)), PAGE_SIZE * 2, "DMI Decoder Finding Table", Region::Access::Read, false, false);
-        SMBIOS::TableHeader* table_v_ptr = (SMBIOS::TableHeader*)region->vaddr().offset(offset_in_page((uintptr_t)table)).as_ptr();
+        auto region = MM.allocate_kernel_region(table.page_base(), PAGE_SIZE * 2, "DMI Decoder Finding Table", Region::Access::Read, false, false);
+        SMBIOS::TableHeader* table_v_ptr = (SMBIOS::TableHeader*)region->vaddr().offset(table.offset_in_page().get()).as_ptr();
 
         if (table_v_ptr->handle == handle) {
             return table;
         }
     }
-    return nullptr;
+    return {};
 }
-SMBIOS::TableHeader* DMIDecoder::get_smbios_physical_table_by_type(u8 table_type)
+PhysicalAddress DMIDecoder::get_smbios_physical_table_by_type(u8 table_type)
 {
 
-    for (auto* table : m_smbios_tables) {
-        if (!table)
+    for (auto table : m_smbios_tables) {
+        if (table.is_null())
             continue;
-        auto region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((uintptr_t)table)), PAGE_ROUND_UP(PAGE_SIZE * 2), "DMI Decoder Finding Table", Region::Access::Read, false, false);
-        SMBIOS::TableHeader* table_v_ptr = (SMBIOS::TableHeader*)region->vaddr().offset(offset_in_page((uintptr_t)table)).as_ptr();
+        auto region = MM.allocate_kernel_region(table.page_base(), PAGE_ROUND_UP(PAGE_SIZE * 2), "DMI Decoder Finding Table", Region::Access::Read, false, false);
+        SMBIOS::TableHeader* table_v_ptr = (SMBIOS::TableHeader*)region->vaddr().offset(table.offset_in_page().get()).as_ptr();
         if (table_v_ptr->type == table_type) {
             return table;
         }
     }
-    return nullptr;
+    return {};
 }
 
 DMIDecoder::DMIDecoder(bool trusted)
     : m_entry32bit_point(find_entry32bit_point())
     , m_entry64bit_point(find_entry64bit_point())
-    , m_structure_table(nullptr)
+    , m_structure_table(PhysicalAddress())
     , m_untrusted(!trusted)
 {
     if (!trusted) {
@@ -211,7 +210,7 @@ DMIDecoder::DMIDecoder(bool trusted)
     initialize_parser();
 }
 
-SMBIOS::EntryPoint64bit* DMIDecoder::find_entry64bit_point()
+PhysicalAddress DMIDecoder::find_entry64bit_point()
 {
     PhysicalAddress paddr = PhysicalAddress(SMBIOS_BASE_SEARCH_ADDR);
     auto region = MM.allocate_kernel_region(paddr, PAGE_ROUND_UP(SMBIOS_SEARCH_AREA_SIZE), "DMI Decoder Entry Point 64 bit Finding", Region::Access::Read, false, false);
@@ -222,14 +221,14 @@ SMBIOS::EntryPoint64bit* DMIDecoder::find_entry64bit_point()
         dbgprintf("DMI Decoder: Looking for 64 bit Entry point @ V 0x%x P 0x%x\n", entry_str, tested_physical_ptr);
 #endif
         if (!strncmp("_SM3_", entry_str, strlen("_SM3_")))
-            return (SMBIOS::EntryPoint64bit*)tested_physical_ptr;
+            return PhysicalAddress((uintptr_t)tested_physical_ptr);
 
         tested_physical_ptr += 16;
     }
-    return nullptr;
+    return {};
 }
 
-SMBIOS::EntryPoint32bit* DMIDecoder::find_entry32bit_point()
+PhysicalAddress DMIDecoder::find_entry32bit_point()
 {
     PhysicalAddress paddr = PhysicalAddress(SMBIOS_BASE_SEARCH_ADDR);
     auto region = MM.allocate_kernel_region(paddr, PAGE_ROUND_UP(SMBIOS_SEARCH_AREA_SIZE), "DMI Decoder Entry Point 32 bit Finding", Region::Access::Read, false, false);
@@ -240,11 +239,11 @@ SMBIOS::EntryPoint32bit* DMIDecoder::find_entry32bit_point()
         dbgprintf("DMI Decoder: Looking for 32 bit Entry point @ V 0x%x P 0x%x\n", entry_str, tested_physical_ptr);
 #endif
         if (!strncmp("_SM_", entry_str, strlen("_SM_")))
-            return (SMBIOS::EntryPoint32bit*)tested_physical_ptr;
+            return PhysicalAddress((uintptr_t)tested_physical_ptr);
 
         tested_physical_ptr += 16;
     }
-    return nullptr;
+    return {};
 }
 
 Vector<SMBIOS::PhysicalMemoryArray*>& DMIDecoder::get_physical_memory_areas()
@@ -260,15 +259,16 @@ bool DMIDecoder::is_reliable()
 u64 DMIDecoder::get_bios_characteristics()
 {
     // FIXME: Make sure we have some mapping here so we don't rely on existing identity mapping...
+    ASSERT_NOT_REACHED();
     ASSERT(m_operable == true);
-    SMBIOS::BIOSInfo* bios_info = (SMBIOS::BIOSInfo*)get_smbios_physical_table_by_type(0);
+    auto* bios_info = (SMBIOS::BIOSInfo*)get_smbios_physical_table_by_type(0).as_ptr();
     ASSERT(bios_info != nullptr);
 
     kprintf("DMIDecoder: BIOS info @ P 0x%x\n", bios_info);
     return bios_info->bios_characteristics;
 }
 
-char* DMIDecoder::get_smbios_string(SMBIOS::TableHeader&, u8)
+char* DMIDecoder::get_smbios_string(PhysicalAddress, u8)
 {
     // FIXME: Implement it...
     // FIXME: Make sure we have some mapping here so we don't rely on existing identity mapping...

Kernel/ACPI/DMIDecoder.h

@@ -1395,31 +1395,31 @@ public:
 
 private:
     void enumerate_smbios_tables();
-    SMBIOS::TableHeader* get_next_physical_table(SMBIOS::TableHeader& p_table);
-    SMBIOS::TableHeader* get_smbios_physical_table_by_handle(u16 handle);
-    SMBIOS::TableHeader* get_smbios_physical_table_by_type(u8 table_type);
-    char* get_smbios_string(SMBIOS::TableHeader& p_table, u8 string_number);
-    size_t get_table_size(SMBIOS::TableHeader& table);
+    PhysicalAddress get_next_physical_table(PhysicalAddress p_table);
+    PhysicalAddress get_smbios_physical_table_by_handle(u16 handle);
+    PhysicalAddress get_smbios_physical_table_by_type(u8 table_type);
+    char* get_smbios_string(PhysicalAddress, u8 string_number);
+    size_t get_table_size(PhysicalAddress);
 
     explicit DMIDecoder(bool trusted);
     void initialize_parser();
 
-    void set_64_bit_entry_initialization_values(SMBIOS::EntryPoint64bit&);
-    void set_32_bit_entry_initialization_values(SMBIOS::EntryPoint32bit&);
+    void set_64_bit_entry_initialization_values(PhysicalAddress);
+    void set_32_bit_entry_initialization_values(PhysicalAddress);
 
-    SMBIOS::EntryPoint32bit* find_entry32bit_point();
-    SMBIOS::EntryPoint64bit* find_entry64bit_point();
+    PhysicalAddress find_entry32bit_point();
+    PhysicalAddress find_entry64bit_point();
 
-    SMBIOS::EntryPoint32bit* m_entry32bit_point;
-    SMBIOS::EntryPoint64bit* m_entry64bit_point;
-    SMBIOS::TableHeader* m_structure_table;
+    PhysicalAddress m_entry32bit_point;
+    PhysicalAddress m_entry64bit_point;
+    PhysicalAddress m_structure_table;
     u32 m_structures_count;
     u32 m_table_length;
     bool m_use_64bit_entry;
     bool m_operable;
     bool m_untrusted;
 
-    SinglyLinkedList<SMBIOS::TableHeader*> m_smbios_tables;
+    SinglyLinkedList<PhysicalAddress> m_smbios_tables;
 };
 
 }