diff options
| author | Brian Gianforcaro <bgianf@serenityos.org> | 2022-03-13 20:07:31 -0700 |
|---|---|---|
| committer | Andreas Kling <kling@serenityos.org> | 2022-03-14 22:30:22 +0100 |
| commit | c0ed656c94ffa11e1949ed2e4cc68469aa0d0cd0 (patch) | |
| tree | 07f2e08b8ed3f01a57eecb2583b6e283fc32d741 /Kernel/Graphics | |
| parent | af50895fa316129b0bf37a4b8f9810b6bc365804 (diff) | |
| download | serenity-c0ed656c94ffa11e1949ed2e4cc68469aa0d0cd0.zip | |
Kernel: Fix buffer overflow in VirtIOGPU create_3d_resource(..)
This code attempts to copy the `Protocol::Resource3DSpecification`
struct into request, starting at `Protocol::ResourceCreate3D::target`
member of the `Protocol::ResourceCreate3D` struct.
The problem is that the `Protocol::Resource3DSpecification` struct
does not having the trailing `u32 padding` that the `ResourceCreate3D`
struct has. Leading to memcopy overrunning the struct and corrupting
32 bits of data trailing the struct.
Found by SonarCloud:
- Memory copy function overflows the destination buffer.
Diffstat (limited to 'Kernel/Graphics')
| -rw-r--r-- | Kernel/Graphics/VirtIOGPU/GPU3DDevice.cpp | 3 | ||||
| -rw-r--r-- | Kernel/Graphics/VirtIOGPU/GraphicsAdapter.cpp | 5 | ||||
| -rw-r--r-- | Kernel/Graphics/VirtIOGPU/Protocol.h | 1 |
3 files changed, 7 insertions, 2 deletions
diff --git a/Kernel/Graphics/VirtIOGPU/GPU3DDevice.cpp b/Kernel/Graphics/VirtIOGPU/GPU3DDevice.cpp index 0367376237..c7b8e34cd9 100644 --- a/Kernel/Graphics/VirtIOGPU/GPU3DDevice.cpp +++ b/Kernel/Graphics/VirtIOGPU/GPU3DDevice.cpp @@ -114,7 +114,8 @@ ErrorOr<void> GPU3DDevice::ioctl(OpenFileDescription& description, unsigned requ .array_size = spec.array_size, .last_level = spec.last_level, .nr_samples = spec.nr_samples, - .flags = spec.flags + .flags = spec.flags, + .padding = 0, }; MutexLocker locker(m_graphics_adapter.operation_lock()); auto resource_id = m_graphics_adapter.create_3d_resource(resource_spec).value(); diff --git a/Kernel/Graphics/VirtIOGPU/GraphicsAdapter.cpp b/Kernel/Graphics/VirtIOGPU/GraphicsAdapter.cpp index 2ec8b461e3..23376d31f5 100644 --- a/Kernel/Graphics/VirtIOGPU/GraphicsAdapter.cpp +++ b/Kernel/Graphics/VirtIOGPU/GraphicsAdapter.cpp @@ -284,7 +284,10 @@ ResourceID GraphicsAdapter::create_3d_resource(Protocol::Resource3DSpecification request.resource_id = resource_id.value(); // TODO: Abstract this out a bit more u32* start_of_copied_fields = &request.target; - memcpy(start_of_copied_fields, &resource_3d_specification, sizeof(Protocol::Resource3DSpecification)); + + // Validate that the sub copy from the resource_3d_specification to the offset of the request fits. + static_assert((sizeof(request) - offsetof(Protocol::ResourceCreate3D, target) == sizeof(resource_3d_specification))); + memcpy(start_of_copied_fields, &resource_3d_specification, sizeof(resource_3d_specification)); synchronous_virtio_gpu_command(start_of_scratch_space(), sizeof(request), sizeof(response)); diff --git a/Kernel/Graphics/VirtIOGPU/Protocol.h b/Kernel/Graphics/VirtIOGPU/Protocol.h index e8c3483185..374fab657c 100644 --- a/Kernel/Graphics/VirtIOGPU/Protocol.h +++ b/Kernel/Graphics/VirtIOGPU/Protocol.h @@ -328,6 +328,7 @@ struct Resource3DSpecification { u32 last_level; u32 nr_samples; u32 flags; + u32 padding; }; } |