Skip to contentSkip to conversation

drm/radeon: integer underflow in radeon_cp_dispatch_texture()

PATCH Applied Dan Carpenter 3 participants 3 messages Dec 23, 2014 – Jan 5, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drm/radeon: integer underflow in radeon_cp_dispatch_texture()

The test:

	if (size > RADEON_MAX_TEXTURE_SIZE) {

"size" is an integer and it's controled by the user so it can be negative and the test can underflow. Later we use "size" in:

	dwords = size / 4;
	...
	RADEON_COPY_MT(buffer, data, (int)(dwords * sizeof(u32)));

It causes memory corruption to copy a negative size buffer.

---
Static checkers complain about the integer overflows here, and there are many real overflows but they appear harmless.

drivers/gpu/drm/radeon/radeon_state.c +1 -1
index 535403e..15aee72 100644
@@ -1703,7 +1703,7 @@ static int radeon_cp_dispatch_texture(struct drm_device * dev,
 	u32 format;
 	u32 *buffer;
 	const u8 __user *data;
-	int size, dwords, tex_width, blit_width, spitch;
+	unsigned int size, dwords, tex_width, blit_width, spitch;
 	u32 height;
 	int i;
 	u32 texpitch, microtile;

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

maintainer
#1 lore raw

Am 23.12.2014 um 10:56 schrieb Dan Carpenter:

Quoted 7 lines

The test:

	if (size > RADEON_MAX_TEXTURE_SIZE) {

"size" is an integer and it's controled by the user so it can be negative and the test can underflow. Later we use "size" in:

Quoted 7 lines
	dwords = size / 4;
	...
	RADEON_COPY_MT(buffer, data, (int)(dwords * sizeof(u32)));

It causes memory corruption to copy a negative size buffer.

This specific line of code is completely deprecated and the patch is just another coffin nail to finally remove it.

But since we can't be sure that it doesn't break any userspace still in use I'm generally ok to apply the patch and it is Reviewed-by: Christian König <[email protected]>

Regards,
Christian.

Quoted 9 lines

---
Static checkers complain about the integer overflows here, and there are many real overflows but they appear harmless.

drivers/gpu/drm/radeon/radeon_state.c +0 -0
index 535403e..15aee72 100644

drivers/gpu/drm/radeon/radeon_state.c static int radeon_cp_dispatch_texture(struct drm_device * dev, · line 1709

Quoted 8 lines
drivers/gpu/drm/radeon/radeon_state.c +1 -1
@@ -1703,7 +1703,7 @@ static int radeon_cp_dispatch_texture(struct drm_device * dev,
  	u32 format;
  	u32 *buffer;
  	const u8 __user *data;
-	int size, dwords, tex_width, blit_width, spitch;
+	unsigned int size, dwords, tex_width, blit_width, spitch;
  	u32 height;
  	int i;
  	u32 texpitch, microtile;

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Applied
#2 lore raw
In reply to Dan Carpenter
Quoted 7 lines · Dan Carpenter

The test:

        if (size > RADEON_MAX_TEXTURE_SIZE) {

"size" is an integer and it's controled by the user so it can be negative and the test can underflow. Later we use "size" in:

Quoted 7 lines
        dwords = size / 4;
        ...
        RADEON_COPY_MT(buffer, data, (int)(dwords * sizeof(u32)));

It causes memory corruption to copy a negative size buffer.

Applied to my fixes tree. thanks!

Alex

Quoted 17 lines

---
Static checkers complain about the integer overflows here, and there are many real overflows but they appear harmless.

drivers/gpu/drm/radeon/radeon_state.c +1 -1
index 535403e..15aee72 100644
@@ -1703,7 +1703,7 @@ static int radeon_cp_dispatch_texture(struct drm_device * dev,
        u32 format;
        u32 *buffer;
        const u8 __user *data;
-       int size, dwords, tex_width, blit_width, spitch;
+       unsigned int size, dwords, tex_width, blit_width, spitch;
        u32 height;
        int i;
        u32 texpitch, microtile;
Quoted 4 lines

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel