Re: [PATCH xf86-video-intel v2] SNA: fix PRIME output support since xserver 1.20

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Nov 15, 2019 at 04:32:47PM +0100, Peter Wu wrote:
> Since "Make PixmapDirtyUpdateRec::src a DrawablePtr" in xserver, the
> "src" pointer might point to the root window (created by the server)
> instead of a pixmap (as created by xf86-video-intel). Use
> get_drawable_pixmap to handle both cases.
> 
> When built with -fsanitize=address, the following test on a hybrid
> graphics laptop will trigger a heap-buffer-overflow error due to
> to_sna_from_pixmap receiving a window instead of a pixmap:
> 
>     xrandr --setprovideroutputsource modesetting Intel
>     xrandr --output DP-1-1 --mode 2560x1440  # should not crash
>     glxgears  # should display gears on both screens
> 
> With nouveau instead of modesetting, it does not crash but the external
> monitor remains blank aside from a mouse cursor. This patch fixes both.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100086
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
> v1: https://lists.freedesktop.org/archives/intel-gfx/2018-August/173522.html
> v2: rebased on current master (2.99.917-893-gbff5eca4), reworded commit.
> 
> This patch has been tested at https://bugs.archlinux.org/task/64238, I
> have additionally tested it with both modesetting and nouveau under
> ASAN, the modesetting ASAN trace for unpatched intel can be found at:
> https://bugs.freedesktop.org/show_bug.cgi?id=100086#c24
> 
> commit 2.99.917-891-g581ddc5d ("sna: Fix compiler warnings due to
> DrawablePtr vs. PixmapPtr") incorporated all compiler warning fixes from
> v1 of this patch, but unfortunately lacks this crucial bugfix.
> ---
>  src/sna/sna_accel.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
> index fa386ff6..ee857a14 100644
> --- a/src/sna/sna_accel.c
> +++ b/src/sna/sna_accel.c
> @@ -17684,10 +17684,10 @@ static void sna_accel_post_damage(struct sna *sna)
>  			continue;
>  
>  #ifdef HAS_DIRTYTRACKING_DRAWABLE_SRC
> -		assert(dirty->src->type == DRAWABLE_PIXMAP);
> +		src = get_drawable_pixmap(dirty->src);
> +#else
> +		src = dirty->src;

Looks sensible enough to me:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  #endif
> -
> -		src = (PixmapPtr)dirty->src;
>  		dst = dirty->slave_dst->master_pixmap;
>  
>  		region.extents.x1 = dirty->x;
> -- 
> 2.23.0

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx