From 385dacce05901833482e23127dd8580db4e134f9 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 29 Apr 2020 09:35:19 +0200 Subject: Kernel: Fix integer overflow in framebuffer resolution handling This made it possible to map the E1000 MMIO range into userspace and mess with the registers. Thanks to @grigoritchy for finding this! Fixes #2015. --- Kernel/Devices/BXVGADevice.cpp | 12 ++++++------ Kernel/Devices/BXVGADevice.h | 18 +++++++++--------- Kernel/Devices/MBVGADevice.cpp | 2 +- Kernel/Devices/MBVGADevice.h | 8 ++++---- 4 files changed, 20 insertions(+), 20 deletions(-) (limited to 'Kernel/Devices') diff --git a/Kernel/Devices/BXVGADevice.cpp b/Kernel/Devices/BXVGADevice.cpp index 47b69f6532..67490d95c9 100644 --- a/Kernel/Devices/BXVGADevice.cpp +++ b/Kernel/Devices/BXVGADevice.cpp @@ -98,7 +98,7 @@ void BXVGADevice::revert_resolution() ASSERT(validate_setup_resolution(m_framebuffer_width, m_framebuffer_height)); } -void BXVGADevice::set_resolution_registers(int width, int height) +void BXVGADevice::set_resolution_registers(size_t width, size_t height) { #ifdef BXVGA_DEBUG dbg() << "BXVGADevice resolution registers set to - " << width << "x" << height; @@ -113,7 +113,7 @@ void BXVGADevice::set_resolution_registers(int width, int height) set_register(VBE_DISPI_INDEX_BANK, 0); } -bool BXVGADevice::test_resolution(int width, int height) +bool BXVGADevice::test_resolution(size_t width, size_t height) { #ifdef BXVGA_DEBUG dbg() << "BXVGADevice resolution test - " << width << "x" << height; @@ -123,9 +123,9 @@ bool BXVGADevice::test_resolution(int width, int height) revert_resolution(); return resolution_changed; } -bool BXVGADevice::set_resolution(int width, int height) +bool BXVGADevice::set_resolution(size_t width, size_t height) { - if (Checked::multiplication_would_overflow(width, height, sizeof(u32))) + if (Checked::multiplication_would_overflow(width, height, sizeof(u32))) return false; if (!test_resolution(width, height)) @@ -140,7 +140,7 @@ bool BXVGADevice::set_resolution(int width, int height) return true; } -bool BXVGADevice::validate_setup_resolution(int width, int height) +bool BXVGADevice::validate_setup_resolution(size_t width, size_t height) { if ((u16)width != get_register(VBE_DISPI_INDEX_XRES) || (u16)height != get_register(VBE_DISPI_INDEX_YRES)) { return false; @@ -148,7 +148,7 @@ bool BXVGADevice::validate_setup_resolution(int width, int height) return true; } -void BXVGADevice::set_y_offset(int y_offset) +void BXVGADevice::set_y_offset(size_t y_offset) { ASSERT(y_offset == 0 || y_offset == m_framebuffer_height); m_y_offset = y_offset; diff --git a/Kernel/Devices/BXVGADevice.h b/Kernel/Devices/BXVGADevice.h index 122c717d93..7e0a620ee5 100644 --- a/Kernel/Devices/BXVGADevice.h +++ b/Kernel/Devices/BXVGADevice.h @@ -56,20 +56,20 @@ private: void set_register(u16 index, u16 value); u16 get_register(u16 index); - bool validate_setup_resolution(int width, int height); + bool validate_setup_resolution(size_t width, size_t height); u32 find_framebuffer_address(); void revert_resolution(); - bool test_resolution(int width, int height); + bool test_resolution(size_t width, size_t height); size_t framebuffer_size_in_bytes() const { return m_framebuffer_pitch * m_framebuffer_height * 2; } - bool set_resolution(int width, int height); - void set_resolution_registers(int width, int height); - void set_y_offset(int); + bool set_resolution(size_t width, size_t height); + void set_resolution_registers(size_t width, size_t height); + void set_y_offset(size_t); PhysicalAddress m_framebuffer_address; - int m_framebuffer_pitch { 0 }; - int m_framebuffer_width { 0 }; - int m_framebuffer_height { 0 }; - int m_y_offset { 0 }; + size_t m_framebuffer_pitch { 0 }; + size_t m_framebuffer_width { 0 }; + size_t m_framebuffer_height { 0 }; + size_t m_y_offset { 0 }; }; } diff --git a/Kernel/Devices/MBVGADevice.cpp b/Kernel/Devices/MBVGADevice.cpp index 31d17f1ead..989dbe6681 100644 --- a/Kernel/Devices/MBVGADevice.cpp +++ b/Kernel/Devices/MBVGADevice.cpp @@ -40,7 +40,7 @@ MBVGADevice& MBVGADevice::the() return *s_the; } -MBVGADevice::MBVGADevice(PhysicalAddress addr, int pitch, int width, int height) +MBVGADevice::MBVGADevice(PhysicalAddress addr, size_t pitch, size_t width, size_t height) : BlockDevice(29, 0) , m_framebuffer_address(addr) , m_framebuffer_pitch(pitch) diff --git a/Kernel/Devices/MBVGADevice.h b/Kernel/Devices/MBVGADevice.h index 1fd7f7dd66..3afb118d5c 100644 --- a/Kernel/Devices/MBVGADevice.h +++ b/Kernel/Devices/MBVGADevice.h @@ -38,7 +38,7 @@ class MBVGADevice final : public BlockDevice { public: static MBVGADevice& the(); - MBVGADevice(PhysicalAddress addr, int pitch, int width, int height); + MBVGADevice(PhysicalAddress addr, size_t pitch, size_t width, size_t height); virtual int ioctl(FileDescription&, unsigned request, unsigned arg) override; virtual KResultOr mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t, int prot, bool shared) override; @@ -55,9 +55,9 @@ private: size_t framebuffer_size_in_bytes() const { return m_framebuffer_pitch * m_framebuffer_height; } PhysicalAddress m_framebuffer_address; - int m_framebuffer_pitch { 0 }; - int m_framebuffer_width { 0 }; - int m_framebuffer_height { 0 }; + size_t m_framebuffer_pitch { 0 }; + size_t m_framebuffer_width { 0 }; + size_t m_framebuffer_height { 0 }; }; } -- cgit v1.2.3