Re: [PATCH xserver v2] damage: Validate source pictures bound to windows before unwrapping

[Michel Dänzer]

On 09/02/17 03:06 AM, Adam Jackson wrote:
> On Tue, 2017-02-07 at 11:57 -0500, Alex Deucher wrote:
>> On Tue, Feb 7, 2017 at 3:38 AM, Michel Dänzer 
>> wrote:
>>> From: Michel Dänzer 
>>>
>>> The lower layers also do this, but no damage may be reported there,
>>> since we unwrap before calling down.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99220
>>> Signed-off-by: Michel Dänzer 
>>
>> Makes sense to me.
>> Reviewed-by: Alex Deucher 
> 
> Mmm. I might like the original patch better, I think I'd rather have
> composite be aware of how damage works than damage be aware of every
> possible way a caller might abuse it. So I reserve the right to flip
> this back the other way at some point in the future, if we ever get
> around to dix-level damage.

I'm not sure which caller you're referring to or how it's abusing
damage, but okay.


> But this does work, and I do give preference to patches that other
> people review instead of things I have to review as the last resort,
> so, merged:

Thanks!


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v2] damage: Validate source pictures bound to windows before unwrapping

[Alex Deucher]

On Wed, Feb 8, 2017 at 1:06 PM, Adam Jackson  wrote:
> On Tue, 2017-02-07 at 11:57 -0500, Alex Deucher wrote:
>> On Tue, Feb 7, 2017 at 3:38 AM, Michel Dänzer 
>> wrote:
>> > From: Michel Dänzer 
>> >
>> > The lower layers also do this, but no damage may be reported there,
>> > since we unwrap before calling down.
>> >
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99220
>> > Signed-off-by: Michel Dänzer 
>>
>> Makes sense to me.
>> Reviewed-by: Alex Deucher 
>
> Mmm. I might like the original patch better, I think I'd rather have
> composite be aware of how damage works than damage be aware of every
> possible way a caller might abuse it. So I reserve the right to flip
> this back the other way at some point in the future, if we ever get
> around to dix-level damage.

No objections from me.  I don't have a strong preference.

Alex

>
> But this does work, and I do give preference to patches that other
> people review instead of things I have to review as the last resort,
> so, merged:
>
> remote: I: patch #137290 updated using rev 
> 38696ea56854e055c31bd2730adfc7c39aa115b0.
> remote: I: 1 patch(es) updated to state Accepted.
> To ssh://git.freedesktop.org/git/xorg/xserver
>1c78bec..38696ea  master -> master
>
> - ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v2] damage: Validate source pictures bound to windows before unwrapping

[Adam Jackson]

On Tue, 2017-02-07 at 11:57 -0500, Alex Deucher wrote:
> On Tue, Feb 7, 2017 at 3:38 AM, Michel Dänzer 
> wrote:
> > From: Michel Dänzer 
> > 
> > The lower layers also do this, but no damage may be reported there,
> > since we unwrap before calling down.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99220
> > Signed-off-by: Michel Dänzer 
> 
> Makes sense to me.
> Reviewed-by: Alex Deucher 

Mmm. I might like the original patch better, I think I'd rather have
composite be aware of how damage works than damage be aware of every
possible way a caller might abuse it. So I reserve the right to flip
this back the other way at some point in the future, if we ever get
around to dix-level damage.

But this does work, and I do give preference to patches that other
people review instead of things I have to review as the last resort,
so, merged:

remote: I: patch #137290 updated using rev 
38696ea56854e055c31bd2730adfc7c39aa115b0.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   1c78bec..38696ea  master -> master

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver v2] damage: Validate source pictures bound to windows before unwrapping

[Alex Deucher]

On Tue, Feb 7, 2017 at 3:38 AM, Michel Dänzer  wrote:
> From: Michel Dänzer 
>
> The lower layers also do this, but no damage may be reported there,
> since we unwrap before calling down.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99220
> Signed-off-by: Michel Dänzer 

Makes sense to me.
Reviewed-by: Alex Deucher 

> ---
>
> v2: Update code comment
>
> This is an alternative to https://patchwork.freedesktop.org/patch/136721/ .
>
>  configure.ac  |  2 +-
>  miext/damage/damage.c | 10 ++
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 95aa297da..4dcf8b5c2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1847,7 +1847,7 @@ if test "x$XNEST" = xyes; then
> if test "x$have_xnest" = xno; then
> AC_MSG_ERROR([Xnest build explicitly requested, but required 
> modules not found.])
> fi
> -   XNEST_LIBS="$FB_LIB $FIXES_LIB $MI_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB 
> $GLX_LIBS $RANDR_LIB $RENDER_LIB $DAMAGE_LIB  $DRI3_LIB $PRESENT_LIB 
> $MIEXT_SYNC_LIB $MIEXT_DAMAGE_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB 
> $XKB_STUB_LIB $COMPOSITE_LIB $MAIN_LIB $DIX_LIB $OS_LIB"
> +   XNEST_LIBS="$FB_LIB $FIXES_LIB $MI_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB 
> $GLX_LIBS $RANDR_LIB $DAMAGE_LIB  $DRI3_LIB $PRESENT_LIB $MIEXT_SYNC_LIB 
> $MIEXT_DAMAGE_LIB $RENDER_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB 
> $XKB_STUB_LIB $COMPOSITE_LIB $MAIN_LIB $DIX_LIB $OS_LIB"
> XNEST_SYS_LIBS="$XNESTMODULES_LIBS $GLX_SYS_LIBS"
> AC_SUBST([XNEST_LIBS])
> AC_SUBST([XNEST_SYS_LIBS])
> diff --git a/miext/damage/damage.c b/miext/damage/damage.c
> index d6a36142d..2fae03fc2 100644
> --- a/miext/damage/damage.c
> +++ b/miext/damage/damage.c
> @@ -34,6 +34,7 @@
>  #include
>  #include
>  #include"mi.h"
> +#include"mipict.h"
>  #include"regionstr.h"
>  #include"globals.h"
>  #include"gcstruct.h"
> @@ -499,6 +500,15 @@ damageComposite(CARD8 op,
>  if (BOX_NOT_EMPTY(box))
>  damageDamageBox(pDst->pDrawable, , pDst->subWindowMode);
>  }
> +/*
> + * Validating a source picture bound to a window may trigger other
> + * composite operations. Do it before unwrapping to make sure damage
> + * is reported correctly.
> + */
> +if (pSrc->pDrawable && WindowDrawable(pSrc->pDrawable->type))
> +miCompositeSourceValidate(pSrc);
> +if (pMask && pMask->pDrawable && WindowDrawable(pMask->pDrawable->type))
> +miCompositeSourceValidate(pMask);
>  unwrap(pScrPriv, ps, Composite);
>  (*ps->Composite) (op,
>pSrc,
> --
> 2.11.0
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver v2] damage: Validate source pictures bound to windows before unwrapping

[Michel Dänzer]

From: Michel Dänzer 

The lower layers also do this, but no damage may be reported there,
since we unwrap before calling down.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99220
Signed-off-by: Michel Dänzer 
---

v2: Update code comment

This is an alternative to https://patchwork.freedesktop.org/patch/136721/ .

 configure.ac  |  2 +-
 miext/damage/damage.c | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 95aa297da..4dcf8b5c2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1847,7 +1847,7 @@ if test "x$XNEST" = xyes; then
if test "x$have_xnest" = xno; then
AC_MSG_ERROR([Xnest build explicitly requested, but required 
modules not found.])
fi
-   XNEST_LIBS="$FB_LIB $FIXES_LIB $MI_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB 
$GLX_LIBS $RANDR_LIB $RENDER_LIB $DAMAGE_LIB  $DRI3_LIB $PRESENT_LIB 
$MIEXT_SYNC_LIB $MIEXT_DAMAGE_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB 
$XKB_STUB_LIB $COMPOSITE_LIB $MAIN_LIB $DIX_LIB $OS_LIB"
+   XNEST_LIBS="$FB_LIB $FIXES_LIB $MI_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB 
$GLX_LIBS $RANDR_LIB $DAMAGE_LIB  $DRI3_LIB $PRESENT_LIB $MIEXT_SYNC_LIB 
$MIEXT_DAMAGE_LIB $RENDER_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB $XKB_STUB_LIB 
$COMPOSITE_LIB $MAIN_LIB $DIX_LIB $OS_LIB"
XNEST_SYS_LIBS="$XNESTMODULES_LIBS $GLX_SYS_LIBS"
AC_SUBST([XNEST_LIBS])
AC_SUBST([XNEST_SYS_LIBS])
diff --git a/miext/damage/damage.c b/miext/damage/damage.c
index d6a36142d..2fae03fc2 100644
--- a/miext/damage/damage.c
+++ b/miext/damage/damage.c
@@ -34,6 +34,7 @@
 #include
 #include
 #include"mi.h"
+#include"mipict.h"
 #include"regionstr.h"
 #include"globals.h"
 #include"gcstruct.h"
@@ -499,6 +500,15 @@ damageComposite(CARD8 op,
 if (BOX_NOT_EMPTY(box))
 damageDamageBox(pDst->pDrawable, , pDst->subWindowMode);
 }
+/*
+ * Validating a source picture bound to a window may trigger other
+ * composite operations. Do it before unwrapping to make sure damage
+ * is reported correctly.
+ */
+if (pSrc->pDrawable && WindowDrawable(pSrc->pDrawable->type))
+miCompositeSourceValidate(pSrc);
+if (pMask && pMask->pDrawable && WindowDrawable(pMask->pDrawable->type))
+miCompositeSourceValidate(pMask);
 unwrap(pScrPriv, ps, Composite);
 (*ps->Composite) (op,
   pSrc,
-- 
2.11.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel