[PATCH] drm/gud: validate GUD_ROTATION_0 is present in supported rotations

[PATCH] drm/gud: validate GUD_ROTATION_0 is present in supported rotations

[Sajal Gupta]

The rotation argument to drm_plane_create_rotation_property() is set to
DRM_MODE_ROTATE_0, and the device reported rotation bitmask is used as
the supported_rotations argument. The driver never validates that
GUD_ROTATION_0 is present, so a device that omits it from its
GUD_PROPERTY_ROTATION triggers the
WARN_ON(rotation & ~supported_rotations) in
drm_plane_create_rotation_property()

Fix this by skipping the creation of rotation property if the device
doesn't have the GUD_ROTATION_0 bit

Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
Reported-by: syzbot+efe2810681f1b065d3a8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=efe2810681f1b065d3a8
Tested-by: syzbot+efe2810681f1b065d3a8@syzkaller.appspotmail.com
Signed-off-by: Sajal Gupta <sajal2005gupta@gmail.com>
---
 drivers/gpu/drm/gud/gud_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
index 89bd6ca36003..3a1b9e2a2eaa 100644
--- a/drivers/gpu/drm/gud/gud_drv.c
+++ b/drivers/gpu/drm/gud/gud_drv.c
@@ -289,6 +289,8 @@ static int gud_plane_add_properties(struct gud_device *gdrm)
 			 * but mask out any additions on future devices.
 			 */
 			val &= GUD_ROTATION_MASK;
+			if (!(val & GUD_ROTATION_0))
+				continue;
 			ret = drm_plane_create_rotation_property(&gdrm->plane,
 								 DRM_MODE_ROTATE_0, val);
 			break;
--
2.55.0

Re: [PATCH] drm/gud: validate GUD_ROTATION_0 is present in supported rotations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] An out-of-bounds heap write occurs in `gud_plane_atomic_check` due to an incorrect array indexing bug. When populating `req->properties` with plane properties, the driver uses `req->properties[num_properties + i]` while simultaneously incrementing `num_properties` inside the loop. This causes the effective index to be `initial_num_properties + 2 * i`, which skips elements and ultimately writes past the end of the allocated array. A malicious USB device can trigger this by advertising multiple plane and connector properties (up to 32 of each), causing the loop to write up to index 94 in an array allocated for 64 elements.
--

--- Patch [1]: [PATCH] drm/gud: validate GUD_ROTATION_0 is present in supported rotations ---
commit 63f4e25c2ff8da9667835a1e92cc53b147ae5f56
Author: Sajal Gupta <sajal2005gupta@gmail.com>

drm/gud: validate GUD_ROTATION_0 is present in supported rotations

The rotation argument to drm_plane_create_rotation_property() is set to
DRM_MODE_ROTATE_0, and the device reported rotation bitmask is used as
the supported_rotations argument. The driver never validates that
GUD_ROTATION_0 is present, so a device that omits it from its
GUD_PROPERTY_ROTATION triggers a warning. Fix this by skipping the creation
of rotation property if the device doesn't have the GUD_ROTATION_0 bit.

Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
Reported-by: syzbot+efe2810681f1b065d3a8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=efe2810681f1b065d3a8
Tested-by: syzbot+efe2810681f1b065d3a8@syzkaller.appspotmail.com
Signed-off-by: Sajal Gupta <sajal2005gupta@gmail.com>

> diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
> index 89bd6ca36003f..3a1b9e2a2eaa1 100644
> --- a/drivers/gpu/drm/gud/gud_drv.c
> +++ b/drivers/gpu/drm/gud/gud_drv.c
[ ... ]

[Severity: Critical]
This is a pre-existing issue, but can this loop write out of bounds?

Looking at gud_plane_atomic_check() in drivers/gpu/drm/gud/gud_pipe.c:

	num_properties = ret;
	for (i = 0; i < gdrm->num_properties; i++) {
		...
		req->properties[num_properties + i].prop = cpu_to_le16(prop);
		req->properties[num_properties + i].val = cpu_to_le64(val);
		num_properties++;
	}

Since num_properties is incremented inside the loop, the index
num_properties + i actually increments by 2 on each iteration.

If a USB device advertises multiple plane and connector properties,
would this skip array elements and eventually write past the end of the
req->properties allocation, causing a heap buffer overflow?

Could this be fixed by just using num_properties as the index instead of
num_properties + i?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821071812.16500-1-sajal2005gupta@gmail.com?part=1