| Message ID | 20191115153247.372989-1-peter@lekensteyn.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [xf86-video-intel,v2] SNA: fix PRIME output support since xserver 1.20 | expand |
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
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 Also Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111976 > 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; > #endif > - > - src = (PixmapPtr)dirty->src; > dst = dirty->slave_dst->master_pixmap; > > region.extents.x1 = dirty->x; > -- > 2.23.0
On Fri, Nov 15, 2019 at 08:14:05PM +0200, Ville Syrjälä wrote: > 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 > > Also > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111976 I marked this bug as duplicate of the former since it is the same issue. About the CI failure (https://lists.freedesktop.org/archives/intel-gfx/2019-November/220187.html), should I be concerned? I can't see what tree it is trying to apply the patch to. Is it actually trying to apply it to xf86-video-intel, or is it trying the Linux kernel instead? > > 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; > > #endif > > - > > - src = (PixmapPtr)dirty->src; > > dst = dirty->slave_dst->master_pixmap; > > > > region.extents.x1 = dirty->x; > > -- > > 2.23.0 > > -- > Ville Syrjälä > Intel
On Sat, Nov 16, 2019 at 05:13:17PM +0100, Peter Wu wrote: > On Fri, Nov 15, 2019 at 08:14:05PM +0200, Ville Syrjälä wrote: > > 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 > > > > Also > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111976 > > I marked this bug as duplicate of the former since it is the same issue. > > About the CI failure > (https://lists.freedesktop.org/archives/intel-gfx/2019-November/220187.html), > should I be concerned? I can't see what tree it is trying to apply the > patch to. Is it actually trying to apply it to xf86-video-intel, or is > it trying the Linux kernel instead? Yeah, I think it's trying to apply it to the kernel. We have no CI for the ddx unfortunately. > > > > 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; > > > #endif > > > - > > > - src = (PixmapPtr)dirty->src; > > > dst = dirty->slave_dst->master_pixmap; > > > > > > region.extents.x1 = dirty->x; > > > -- > > > 2.23.0 > > > > -- > > Ville Syrjälä > > Intel
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; #endif - - src = (PixmapPtr)dirty->src; dst = dirty->slave_dst->master_pixmap; region.extents.x1 = dirty->x;
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(-)