[Mesa-dev] [PATCH] svga: fix sid corruption in vmw_drm_surface_from_handle()

Van Der Wath, DanielX J danielx.j.van.der.wath at intel.com
Fri Feb 20 01:32:53 PST 2015


Thanks for looking at this. I tried adding the checks you suggested, but it turns out the problem I'm seeing was actually caused by a mismatch between the kernel and user side versions of the SVGA3dSize struct (the kernel version is bigger, and so when it was copied back into user space it was overwriting other data, handle in my case). 

This patch was only correcting the corruption after it happened, so I've submitted another one that pads SVGA3dSize to the same size as drm_vmw_size, to prevent the corruption from happening in the first place.

Cheers,
Daniel

On Tue, Feb 03, 2015 at 03:16:51PM +0000, danielx.j.van.der.wath at intel.com wrote:
> From: Daniel van der Wath <danielx.j.van.der.wath at intel.com>
> 
> The value stored in "handle" is trashed before being copied into the
> surface's sid. Use the original value from "whandle->handle" instead.
> This fixes a bug with Weston running on VMWare, where SVGA3D_SetRenderTarget()
> would fail and prevent anything from being drawn on screen.
> 
> Reviewed-by: Satyeshwar Singh <satyeshwar.singh at intel.com>
> ---
>  src/gallium/winsys/svga/drm/vmw_screen_dri.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/gallium/winsys/svga/drm/vmw_screen_dri.c b/src/gallium/winsys/svga/drm/vmw_screen_dri.c
> index 79a1b3e..0f796c4 100644
> --- a/src/gallium/winsys/svga/drm/vmw_screen_dri.c
> +++ b/src/gallium/winsys/svga/drm/vmw_screen_dri.c
> @@ -319,7 +319,7 @@ vmw_drm_surface_from_handle(struct svga_winsys_screen *sws,
>      pipe_reference_init(&vsrf->refcnt, 1);
>      p_atomic_set(&vsrf->validated, 0);
>      vsrf->screen = vws;
> -    vsrf->sid = handle;
> +    vsrf->sid = whandle->handle;

This doesn't look right because the patch will allow referencing
a surface that has been destroyed.

The problem is probably from an unnecessary call to
vmw_ioctl_surface_destroy() earlier in the function.

Instead of the current patch, can you try adding a check for
"if (vws->ioctl.have_drm_2_6)" at line 262 covering the entire
DRM_API_HANDLE_TYPE_FD case, and also at line 286 for the existing
IF condition?

If you like, you can take a look at commit 2f6fcd65 for ideas.

Please give it a try and let me know.

Thanks!

Sinclair


More information about the mesa-dev mailing list