2024-04-25 19:28:00

by Zack Rusin

Subject: [PATCH] drm/vmwgfx: Fix invalid reads in fence signaled events

Correctly set the length of the drm_event to the size of the structure
that's actually used.

The length of the drm_event was set to the parent structure instead of
to the drm_vmw_event_fence which is supposed to be read. drm_read
uses the length parameter to copy the event to the user space thus
resuling in oob reads.

Signed-off-by: Zack Rusin <[email protected]>
Fixes: 8b7de6aa8468 ("vmwgfx: Rework fence event action")
Reported-by: [email protected] # ZDI-CAN-23566
Cc: David Airlie <[email protected]>
CC: Daniel Vetter <[email protected]>
Cc: Zack Rusin <[email protected]>
Cc: Broadcom internal kernel review list <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: <[email protected]> # v3.4+
---
drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
index 2a0cda324703..5efc6a766f64 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
@@ -991,7 +991,7 @@ static int vmw_event_fence_action_create(struct drm_file *file_priv,
}

event->event.base.type = DRM_VMW_EVENT_FENCE_SIGNALED;
- event->event.base.length = sizeof(*event);
+ event->event.base.length = sizeof(event->event);
event->event.user_data = user_data;

ret = drm_event_reserve_init(dev, file_priv, &event->base, &event->event.base);
--
2.40.1

2024-04-26 02:31:15

by Maaz Mombasawala

Subject: Re: [PATCH] drm/vmwgfx: Fix invalid reads in fence signaled events

On 4/25/24 12:27, Zack Rusin wrote:
> Correctly set the length of the drm_event to the size of the structure
> that's actually used.
>
> The length of the drm_event was set to the parent structure instead of
> to the drm_vmw_event_fence which is supposed to be read. drm_read
> uses the length parameter to copy the event to the user space thus
> resuling in oob reads.
>
> Signed-off-by: Zack Rusin <[email protected]>
> Fixes: 8b7de6aa8468 ("vmwgfx: Rework fence event action")
> Reported-by: [email protected] # ZDI-CAN-23566
> Cc: David Airlie <[email protected]>
> CC: Daniel Vetter <[email protected]>
> Cc: Zack Rusin <[email protected]>
> Cc: Broadcom internal kernel review list <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: <[email protected]> # v3.4+
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> index 2a0cda324703..5efc6a766f64 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> @@ -991,7 +991,7 @@ static int vmw_event_fence_action_create(struct drm_file *file_priv,
> }
>
> event->event.base.type = DRM_VMW_EVENT_FENCE_SIGNALED;
> - event->event.base.length = sizeof(*event);
> + event->event.base.length = sizeof(event->event);
> event->event.user_data = user_data;
>
> ret = drm_event_reserve_init(dev, file_priv, &event->base, &event->event.base);

LGTM!

Reviewed-by: Maaz Mombasawala <[email protected]>

Thanks,

Maaz Mombasawala <[email protected]>

2024-04-26 08:21:09

by Martin Krastev

Subject: Re: [PATCH] drm/vmwgfx: Fix invalid reads in fence signaled events

LGTM!

Reviewed-by: Martin Krastev <[email protected]>

Regards,
Martin

On Thu, Apr 25, 2024 at 10:27 PM Zack Rusin <[email protected]> wrote:
>
> Correctly set the length of the drm_event to the size of the structure
> that's actually used.
>
> The length of the drm_event was set to the parent structure instead of
> to the drm_vmw_event_fence which is supposed to be read. drm_read
> uses the length parameter to copy the event to the user space thus
> resuling in oob reads.
>
> Signed-off-by: Zack Rusin <[email protected]>
> Fixes: 8b7de6aa8468 ("vmwgfx: Rework fence event action")
> Reported-by: [email protected] # ZDI-CAN-23566
> Cc: David Airlie <[email protected]>
> CC: Daniel Vetter <[email protected]>
> Cc: Zack Rusin <[email protected]>
> Cc: Broadcom internal kernel review list <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: <[email protected]> # v3.4+
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> index 2a0cda324703..5efc6a766f64 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> @@ -991,7 +991,7 @@ static int vmw_event_fence_action_create(struct drm_file *file_priv,
> }
>
> event->event.base.type = DRM_VMW_EVENT_FENCE_SIGNALED;
> - event->event.base.length = sizeof(*event);
> + event->event.base.length = sizeof(event->event);
> event->event.user_data = user_data;
>
> ret = drm_event_reserve_init(dev, file_priv, &event->base, &event->event.base);
> --
> 2.40.1
>