Ver Fonte

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.
Andreas Kling há 5 anos atrás
pai
commit
385dacce05

+ 6 - 6
Kernel/Devices/BXVGADevice.cpp

@@ -98,7 +98,7 @@ void BXVGADevice::revert_resolution()
     ASSERT(validate_setup_resolution(m_framebuffer_width, m_framebuffer_height));
     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
 #ifdef BXVGA_DEBUG
     dbg() << "BXVGADevice resolution registers set to - " << width << "x" << height;
     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);
     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
 #ifdef BXVGA_DEBUG
     dbg() << "BXVGADevice resolution test - " << width << "x" << height;
     dbg() << "BXVGADevice resolution test - " << width << "x" << height;
@@ -123,9 +123,9 @@ bool BXVGADevice::test_resolution(int width, int height)
     revert_resolution();
     revert_resolution();
     return resolution_changed;
     return resolution_changed;
 }
 }
-bool BXVGADevice::set_resolution(int width, int height)
+bool BXVGADevice::set_resolution(size_t width, size_t height)
 {
 {
-    if (Checked<int>::multiplication_would_overflow(width, height, sizeof(u32)))
+    if (Checked<size_t>::multiplication_would_overflow(width, height, sizeof(u32)))
         return false;
         return false;
 
 
     if (!test_resolution(width, height))
     if (!test_resolution(width, height))
@@ -140,7 +140,7 @@ bool BXVGADevice::set_resolution(int width, int height)
     return true;
     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)) {
     if ((u16)width != get_register(VBE_DISPI_INDEX_XRES) || (u16)height != get_register(VBE_DISPI_INDEX_YRES)) {
         return false;
         return false;
@@ -148,7 +148,7 @@ bool BXVGADevice::validate_setup_resolution(int width, int height)
     return true;
     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);
     ASSERT(y_offset == 0 || y_offset == m_framebuffer_height);
     m_y_offset = y_offset;
     m_y_offset = y_offset;

+ 9 - 9
Kernel/Devices/BXVGADevice.h

@@ -56,20 +56,20 @@ private:
 
 
     void set_register(u16 index, u16 value);
     void set_register(u16 index, u16 value);
     u16 get_register(u16 index);
     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();
     u32 find_framebuffer_address();
     void revert_resolution();
     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; }
     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;
     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 };
 };
 };
 
 
 }
 }

+ 1 - 1
Kernel/Devices/MBVGADevice.cpp

@@ -40,7 +40,7 @@ MBVGADevice& MBVGADevice::the()
     return *s_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)
     : BlockDevice(29, 0)
     , m_framebuffer_address(addr)
     , m_framebuffer_address(addr)
     , m_framebuffer_pitch(pitch)
     , m_framebuffer_pitch(pitch)

+ 4 - 4
Kernel/Devices/MBVGADevice.h

@@ -38,7 +38,7 @@ class MBVGADevice final : public BlockDevice {
 public:
 public:
     static MBVGADevice& the();
     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 int ioctl(FileDescription&, unsigned request, unsigned arg) override;
     virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t, int prot, bool shared) override;
     virtual KResultOr<Region*> 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; }
     size_t framebuffer_size_in_bytes() const { return m_framebuffer_pitch * m_framebuffer_height; }
 
 
     PhysicalAddress m_framebuffer_address;
     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 };
 };
 };
 
 
 }
 }

+ 3 - 3
Libraries/LibC/sys/ioctl_numbers.h

@@ -36,9 +36,9 @@ struct winsize {
 };
 };
 
 
 struct FBResolution {
 struct FBResolution {
-    int pitch;
-    int width;
-    int height;
+    unsigned pitch;
+    unsigned width;
+    unsigned height;
 };
 };
 
 
 __END_DECLS
 __END_DECLS