Re: [xserver PATCH] shm: Also censor images returned by ShmGetImage

[Andrew Eikum]

On Fri, Jul 15, 2016 at 10:57:04AM -0400, Adam Jackson wrote:
> On Wed, 2016-07-13 at 10:57 -0500, Andrew Eikum wrote:
> > Ping, anyone had a chance to look at this?
> 
> remote: I: patch #97386 updated using rev 
> 4926845a57fa8b53e18ea7d3434bf5539e9b7782.
> remote: I: 1 patch(es) updated to state Accepted.
> To ssh://git.freedesktop.org/git/xorg/xserver
>9fcb554..4926845  master -> master
> 

Thanks!

Andrew
___
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: [xserver PATCH] shm: Also censor images returned by ShmGetImage

[Andrew Eikum]

Ping, anyone had a chance to look at this?

Thanks,
Andrew

On Wed, Jul 06, 2016 at 02:13:09PM -0500, Andrew Eikum wrote:
> We currently censor images from dix's GetImage, but not from
> ShmGetImage. This is a method to bypass XACE, creating a potential
> leak. We should censor in both methods.
> 
> Signed-off-by: Andrew Eikum <aei...@codeweavers.com>
> ---
>  Xext/shm.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Xext/shm.c b/Xext/shm.c
> index 0a44b76..0557538 100644
> --- a/Xext/shm.c
> +++ b/Xext/shm.c
> @@ -618,6 +618,7 @@ ProcShmGetImage(ClientPtr client)
>  xShmGetImageReply xgi;
>  ShmDescPtr shmdesc;
>  VisualID visual = None;
> +RegionPtr pVisibleRegion = NULL;
>  int rc;
>  
>  REQUEST(xShmGetImageReq);
> @@ -649,6 +650,9 @@ ProcShmGetImage(ClientPtr client)
> wBorderWidth((WindowPtr) pDraw) + (int) pDraw->height)
>  return BadMatch;
>  visual = wVisual(((WindowPtr) pDraw));
> +pVisibleRegion = NotClippedByChildren((WindowPtr) pDraw);
> +if (pVisibleRegion)
> +RegionTranslate(pVisibleRegion, -pDraw->x, -pDraw->y);
>  }
>  else {
>  if (stuff->x < 0 ||
> @@ -685,6 +689,11 @@ ProcShmGetImage(ClientPtr client)
>   stuff->width, stuff->height,
>   stuff->format, stuff->planeMask,
>   shmdesc->addr + stuff->offset);
> +if (pVisibleRegion)
> +XaceCensorImage(client, pVisibleRegion,
> +PixmapBytePad(stuff->width, pDraw->depth), pDraw,
> +stuff->x, stuff->y, stuff->width, stuff->height,
> +stuff->format, shmdesc->addr + stuff->offset);
>  }
>  else {
>  
> @@ -696,11 +705,19 @@ ProcShmGetImage(ClientPtr client)
>   stuff->width, stuff->height,
>   stuff->format, plane,
>   shmdesc->addr + length);
> +if (pVisibleRegion)
> +XaceCensorImage(client, pVisibleRegion,
> +BitmapBytePad(stuff->width), pDraw,
> +stuff->x, stuff->y, stuff->width, stuff->height,
> +stuff->format, shmdesc->addr + length);
>  length += lenPer;
>  }
>  }
>  }
>  
> +if (pVisibleRegion)
> +RegionDestroy(pVisibleRegion);
> +
>  if (client->swapped) {
>  swaps();
>  swapl();
> -- 
> 2.9.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

[xserver PATCH] shm: Also censor images returned by ShmGetImage

[Andrew Eikum]

We currently censor images from dix's GetImage, but not from
ShmGetImage. This is a method to bypass XACE, creating a potential
leak. We should censor in both methods.

Signed-off-by: Andrew Eikum <aei...@codeweavers.com>
---
 Xext/shm.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/Xext/shm.c b/Xext/shm.c
index 0a44b76..0557538 100644
--- a/Xext/shm.c
+++ b/Xext/shm.c
@@ -618,6 +618,7 @@ ProcShmGetImage(ClientPtr client)
 xShmGetImageReply xgi;
 ShmDescPtr shmdesc;
 VisualID visual = None;
+RegionPtr pVisibleRegion = NULL;
 int rc;
 
 REQUEST(xShmGetImageReq);
@@ -649,6 +650,9 @@ ProcShmGetImage(ClientPtr client)
wBorderWidth((WindowPtr) pDraw) + (int) pDraw->height)
 return BadMatch;
 visual = wVisual(((WindowPtr) pDraw));
+pVisibleRegion = NotClippedByChildren((WindowPtr) pDraw);
+if (pVisibleRegion)
+RegionTranslate(pVisibleRegion, -pDraw->x, -pDraw->y);
 }
 else {
 if (stuff->x < 0 ||
@@ -685,6 +689,11 @@ ProcShmGetImage(ClientPtr client)
  stuff->width, stuff->height,
  stuff->format, stuff->planeMask,
  shmdesc->addr + stuff->offset);
+if (pVisibleRegion)
+XaceCensorImage(client, pVisibleRegion,
+PixmapBytePad(stuff->width, pDraw->depth), pDraw,
+stuff->x, stuff->y, stuff->width, stuff->height,
+stuff->format, shmdesc->addr + stuff->offset);
 }
 else {
 
@@ -696,11 +705,19 @@ ProcShmGetImage(ClientPtr client)
  stuff->width, stuff->height,
  stuff->format, plane,
  shmdesc->addr + length);
+if (pVisibleRegion)
+XaceCensorImage(client, pVisibleRegion,
+BitmapBytePad(stuff->width), pDraw,
+stuff->x, stuff->y, stuff->width, stuff->height,
+stuff->format, shmdesc->addr + length);
 length += lenPer;
 }
 }
 }
 
+if (pVisibleRegion)
+RegionDestroy(pVisibleRegion);
+
 if (client->swapped) {
 swaps();
 swapl();
-- 
2.9.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

Re: [PATCH xserver] Xext: fix build with --disable-xace

[Andrew Eikum]

Makes sense to me. Apologies for breaking your build.

Signed-off-by: Andrew Eikum <aei...@codeweavers.com>

On Tue, Oct 27, 2015 at 01:23:13PM +0100, Julien Cristau wrote:
> Regression from 990cf5b2828f73dc7a07f1e38f608af39acfd81d
> 
> Signed-off-by: Julien Cristau <jcris...@debian.org>
> Cc: Andrew Eikum <aei...@codeweavers.com>
> Cc: Peter Hutterer <peter.hutte...@who-t.net>
> ---
>  Xext/xace.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Xext/xace.h b/Xext/xace.h
> index 3303f76..6a8d0c4 100644
> --- a/Xext/xace.h
> +++ b/Xext/xace.h
> @@ -112,6 +112,7 @@ extern _X_EXPORT void XaceCensorImage(ClientPtr client,
>  
>  #ifdef __GNUC__
>  #define XaceHook(args...) Success
> +#define XaceHookIsSet(args...) 0
>  #define XaceHookDispatch(args...) Success
>  #define XaceHookPropertyAccess(args...) Success
>  #define XaceHookSelectionAccess(args...) Success
> @@ -119,6 +120,7 @@ extern _X_EXPORT void XaceCensorImage(ClientPtr client,
>  #define XaceCensorImage(args...) { ; }
>  #else
>  #define XaceHook(...) Success
> +#define XaceHookIsSet(...) 0
>  #define XaceHookDispatch(...) Success
>  #define XaceHookPropertyAccess(...) Success
>  #define XaceHookSelectionAccess(...) Success
> -- 
> 2.6.1
> 
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[xorg-docs PATCH 1/2] Note that the XaceKeyAvailRec's count field is unused

[Andrew Eikum]

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---
 specs/Xserver/XACE-Spec.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/specs/Xserver/XACE-Spec.xml b/specs/Xserver/XACE-Spec.xml
index 7be9e1d..a416444 100644
--- a/specs/Xserver/XACE-Spec.xml
+++ b/specs/Xserver/XACE-Spec.xml
@@ -1030,7 +1030,7 @@
  and a structfieldcount/structfield field of type 
typeint/type./para
  paraThe structfieldevent/structfield field refers to the 
keyboard event, typically a literalKeyPress/literal or 
literalKeyRelease/literal./para
  paraThe structfieldkeybd/structfield field refers to the input 
device that generated the event./para
- paraThe structfieldcount/structfield field is the number of 
repetitions of the event (not 100\% sure of this at present, however)./para
+ paraThe structfieldcount/structfield field is currently 
unused./para
  paraThis hook has no return value./para
/section
 
-- 
2.4.5


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

[xorg-docs PATCH 2/2] The auditing hooks use the XaceAuditRec struct

[Andrew Eikum]

Signed-off-by: Andrew Eikum aei...@codeweavers.com

---

Just a typo fix.

 specs/Xserver/XACE-Spec.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/specs/Xserver/XACE-Spec.xml b/specs/Xserver/XACE-Spec.xml
index a416444..07f3166 100644
--- a/specs/Xserver/XACE-Spec.xml
+++ b/specs/Xserver/XACE-Spec.xml
@@ -1036,7 +1036,7 @@
 
section id=audit_avail_hook
  titleAuditing Hooks/title
- paraTwo hooks provide basic auditing support.  The begin hook is 
called immediately before an incoming client request is dispatched and before 
the dispatch hook is called (refer to xref linkend=core_dispatch_hook/).  
The end hook is called immediately after the processing of the request has 
finished.  The hook argument is a pointer to a structure of type 
typeXaceKeyAvailRec/type.  This structure contains a
+ paraTwo hooks provide basic auditing support.  The begin hook is 
called immediately before an incoming client request is dispatched and before 
the dispatch hook is called (refer to xref linkend=core_dispatch_hook/).  
The end hook is called immediately after the processing of the request has 
finished.  The hook argument is a pointer to a structure of type 
typeXaceAuditRec/type.  This structure contains a
  structfieldclient/structfield field of type 
typeClientPtr/type,
  and a structfieldrequestResult/structfield field of type 
typeint/type./para
  paraThe structfieldclient/structfield field refers to client 
making the request./para
-- 
2.4.5

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

Re: [xserver PATCH 2/2] dix: Send KeyPress and KeyRelease events to the XACE_KEY_AVAIL hook

[Andrew Eikum]

On Wed, Jun 10, 2015 at 02:14:51PM +1000, Peter Hutterer wrote:
 On Thu, Jun 04, 2015 at 01:24:53PM -0500, Andrew Eikum wrote:
  While it's documented in the XACE spec, the XACE_KEY_AVAIL hook is
  currently never actually invoked by the xserver.
  
  This hook was added in 13c6713c82 (25 Aug 2006), but as the keyboard
  processing was moved into XKB, the hook was forgotten and silently
  dropped. The code calling this hook was removed by 7af53799c (4 Jan
  2009), but it was probably already unused before that.
  
  This patch re-adds support for this hook. The count hook parameter is
  unused.
  
  Signed-off-by: Andrew Eikum aei...@codeweavers.com
  ---
  
  Pending approval, updates to the XACE spec documentation for the count
  parameter, and to warn about duplicate events, will follow.
 
 thanks, both merged.
 

Thanks! Is this pushed someplace? I don't see it at
http://cgit.freedesktop.org/xorg/xserver/

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

[xserver PATCH 2/2] dix: Send KeyPress and KeyRelease events to the XACE_KEY_AVAIL hook

[Andrew Eikum]

While it's documented in the XACE spec, the XACE_KEY_AVAIL hook is
currently never actually invoked by the xserver.

This hook was added in 13c6713c82 (25 Aug 2006), but as the keyboard
processing was moved into XKB, the hook was forgotten and silently
dropped. The code calling this hook was removed by 7af53799c (4 Jan
2009), but it was probably already unused before that.

This patch re-adds support for this hook. The count hook parameter is
unused.

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---

Pending approval, updates to the XACE spec documentation for the count
parameter, and to warn about duplicate events, will follow.
---
 Xi/exevents.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/Xi/exevents.c b/Xi/exevents.c
index 1c586d0..cd33f94 100644
--- a/Xi/exevents.c
+++ b/Xi/exevents.c
@@ -1730,6 +1730,18 @@ ProcessDeviceEvent(InternalEvent *ev, DeviceIntPtr 
device)
 break;
 }
 
+/* send KeyPress and KeyRelease events to XACE plugins */
+if (XaceHookIsSet(XACE_KEY_AVAIL) 
+(event-type == ET_KeyPress || event-type == ET_KeyRelease)) {
+xEvent *core;
+int count;
+
+if (EventToCore(ev, core, count) == Success  count  0) {
+XaceHook(XACE_KEY_AVAIL, core, device, 0);
+free(core);
+}
+}
+
 if (DeviceEventCallback  !syncEvents.playingEvents) {
 DeviceEventInfoRec eventinfo;
 SpritePtr pSprite = device-spriteInfo-sprite;
-- 
2.4.2

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

[xserver PATCH 1/2] xace: Add XaceHookIsSet helper function

[Andrew Eikum]

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---
 Xext/xace.c | 15 +++
 Xext/xace.h |  3 +++
 2 files changed, 18 insertions(+)

diff --git a/Xext/xace.c b/Xext/xace.c
index d77b312..b3c67f6 100644
--- a/Xext/xace.c
+++ b/Xext/xace.c
@@ -213,6 +213,21 @@ XaceHook(int hook, ...)
 return prv ? *prv : Success;
 }
 
+/* XaceHookIsSet
+ *
+ * Utility function to determine whether there are any callbacks listening on a
+ * particular XACE hook.
+ *
+ * Returns non-zero if there is a callback, zero otherwise.
+ */
+int
+XaceHookIsSet(int hook)
+{
+if (hook  0 || hook = XACE_NUM_HOOKS)
+return 0;
+return XaceHooks[hook] != NULL;
+}
+
 /* XaceCensorImage
  *
  * Called after pScreen-GetImage to prevent pieces or trusted windows from
diff --git a/Xext/xace.h b/Xext/xace.h
index 5e6cb04..3303f76 100644
--- a/Xext/xace.h
+++ b/Xext/xace.h
@@ -65,6 +65,9 @@ extern _X_EXPORT int XaceHook(int /*hook */ ,
   ...   /*appropriate args for hook */
 );
 
+/* determine whether any callbacks are present for the XACE hook */
+extern _X_EXPORT int XaceHookIsSet(int hook);
+
 /* Special-cased hook functions
  */
 extern _X_EXPORT int XaceHookDispatch(ClientPtr ptr, int major);
-- 
2.4.2


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

Re: [PATCH try2] dix: Send KeyPress and KeyRelease events to the XACE_KEY_AVAIL hook

[Andrew Eikum]

Hey, anyone have any comments on this patch?

Thanks,
Andrew

On Fri, May 15, 2015 at 03:10:13PM -0500, Andrew Eikum wrote:
 While it's documented in the XACE spec, the XACE_KEY_AVAIL hook is
 currently never actually invoked by the xserver.
 
 This hook was added in 13c6713c82 (25 Aug 2006), but as the keyboard
 processing was moved into XKB, the hook was forgotten and silently
 dropped. The code calling this hook was removed by 7af53799c (4 Jan
 2009), but it was probably already unused before that.
 
 This patch re-adds support for this hook. The count hook parameter is
 unused.
 
 Signed-off-by: Andrew Eikum aei...@codeweavers.com
 ---
 
 Change from previous patch: Daniel S on IRC suggested placing the hook
 into ProcessDeviceEvent, as that's where the X server does most of the
 actual event prep and handling before dispatch to clients.
 
 I noticed while testing this that my XACE plugin got multiple calls for
 a single physical keypress. One came from the physical keyboard device,
 and another from the Virtual core keyboard. I thought about doing some
 sort of filtering to prevent duplicate events, but I think it's better
 to ask the XACE plugin to do its own discrimination based on the device.
 
 Pending approval, updates to the XACE spec documentation for the count
 parameter, and to warn about duplicate events, will follow.
 ---
  Xi/exevents.c | 12 
  1 file changed, 12 insertions(+)
 
 diff --git a/Xi/exevents.c b/Xi/exevents.c
 index 1c586d0..859c29f 100644
 --- a/Xi/exevents.c
 +++ b/Xi/exevents.c
 @@ -1730,6 +1730,18 @@ ProcessDeviceEvent(InternalEvent *ev, DeviceIntPtr 
 device)
  break;
  }
  
 +/* send KeyPress and KeyRelease events to XACE plugins */
 +if (XaceHooks[XACE_KEY_AVAIL] 
 +(event-type == ET_KeyPress || event-type == ET_KeyRelease)) {
 +xEvent *core;
 +int count;
 +
 +if (EventToCore(ev, core, count) == Success) {
 +XaceHook(XACE_KEY_AVAIL, core, device, 0);
 +free(core);
 +}
 +}
 +
  if (DeviceEventCallback  !syncEvents.playingEvents) {
  DeviceEventInfoRec eventinfo;
  SpritePtr pSprite = device-spriteInfo-sprite;
 -- 
 2.4.0
 
 ___
 xorg-devel@lists.x.org: X.Org development
 Archives: http://lists.x.org/archives/xorg-devel
 Info: http://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH try2] dix: Send KeyPress and KeyRelease events to the XACE_KEY_AVAIL hook

[Andrew Eikum]

While it's documented in the XACE spec, the XACE_KEY_AVAIL hook is
currently never actually invoked by the xserver.

This hook was added in 13c6713c82 (25 Aug 2006), but as the keyboard
processing was moved into XKB, the hook was forgotten and silently
dropped. The code calling this hook was removed by 7af53799c (4 Jan
2009), but it was probably already unused before that.

This patch re-adds support for this hook. The count hook parameter is
unused.

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---

Change from previous patch: Daniel S on IRC suggested placing the hook
into ProcessDeviceEvent, as that's where the X server does most of the
actual event prep and handling before dispatch to clients.

I noticed while testing this that my XACE plugin got multiple calls for
a single physical keypress. One came from the physical keyboard device,
and another from the Virtual core keyboard. I thought about doing some
sort of filtering to prevent duplicate events, but I think it's better
to ask the XACE plugin to do its own discrimination based on the device.

Pending approval, updates to the XACE spec documentation for the count
parameter, and to warn about duplicate events, will follow.
---
 Xi/exevents.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/Xi/exevents.c b/Xi/exevents.c
index 1c586d0..859c29f 100644
--- a/Xi/exevents.c
+++ b/Xi/exevents.c
@@ -1730,6 +1730,18 @@ ProcessDeviceEvent(InternalEvent *ev, DeviceIntPtr 
device)
 break;
 }
 
+/* send KeyPress and KeyRelease events to XACE plugins */
+if (XaceHooks[XACE_KEY_AVAIL] 
+(event-type == ET_KeyPress || event-type == ET_KeyRelease)) {
+xEvent *core;
+int count;
+
+if (EventToCore(ev, core, count) == Success) {
+XaceHook(XACE_KEY_AVAIL, core, device, 0);
+free(core);
+}
+}
+
 if (DeviceEventCallback  !syncEvents.playingEvents) {
 DeviceEventInfoRec eventinfo;
 SpritePtr pSprite = device-spriteInfo-sprite;
-- 
2.4.0

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

Re: [xserver PATCH] dix: Send KeyPress and KeyRelease events to the XACE_KEY_AVAIL hook

[Andrew Eikum]

Ping. I know XACE doesn't get much work these days, but can someone
take a look at this? Is this an appropriate place to monitor all key
events?

Thanks for looking,
Andrew

On Wed, Apr 22, 2015 at 10:48:59AM -0500, Andrew Eikum wrote:
 While it's documented in the XACE spec, the XACE_KEY_AVAIL hook is
 currently never actually invoked by the xserver.
 
 This hook was added in 13c6713c82 (25 Aug 2006), but as the keyboard
 processing was moved into XKB, the hook was forgotten and silently
 dropped. The code calling this hook was removed by 7af53799c (4 Jan
 2009), but it was probably already unused before that.
 
 This patch re-adds support for this hook. The count hook parameter is
 unused.
 
 Signed-off-by: Andrew Eikum aei...@codeweavers.com
 ---
 
 I'm not intimately familiar with the xserver input code, but this seems
 like a logical place to invoke this XACE hook. The various keyboard
 devices call QueueKeyboardEvents, which places the events into a generic
 input queue for sending to clients. So, I think this makes a decent
 bottleneck through which we can expect all keyboard input events to
 flow.
 
 Updates to the XACE spec documentation for the count parameter will
 follow, pending acceptance.
 
 
  dix/getevents.c | 18 +-
  1 file changed, 17 insertions(+), 1 deletion(-)
 
 diff --git a/dix/getevents.c b/dix/getevents.c
 index d0a87f7..e18248f 100644
 --- a/dix/getevents.c
 +++ b/dix/getevents.c
 @@ -51,6 +51,7 @@
  #include inpututils.h
  #include mi.h
  #include windowstr.h
 +#include xace.h
  
  #include X11/extensions/XKBproto.h
  #include xkbsrv.h
 @@ -1058,9 +1059,24 @@ void
  QueueKeyboardEvents(DeviceIntPtr device, int type,
  int keycode)
  {
 -int nevents;
 +int nevents, i;
  
  nevents = GetKeyboardEvents(InputEventList, device, type, keycode);
 +
 +/* send KeyPress and KeyRelease events to XACE plugins */
 +for (i = 0; i  nevents; i++) {
 +if (InputEventList[i].any.type == ET_KeyPress ||
 +InputEventList[i].any.type == ET_KeyRelease) {
 +xEvent *core;
 +int count;
 +
 +if (EventToCore(InputEventList[i], core, count) == Success) {
 +XaceHook(XACE_KEY_AVAIL, core, device, 0);
 +free(core);
 +}
 +}
 +}
 +
  queueEventList(device, InputEventList, nevents);
  }
  
 -- 
 2.3.5
 
 ___
 xorg-devel@lists.x.org: X.Org development
 Archives: http://lists.x.org/archives/xorg-devel
 Info: http://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[xserver PATCH] dix: Send KeyPress and KeyRelease events to the XACE_KEY_AVAIL hook

[Andrew Eikum]

While it's documented in the XACE spec, the XACE_KEY_AVAIL hook is
currently never actually invoked by the xserver.

This hook was added in 13c6713c82 (25 Aug 2006), but as the keyboard
processing was moved into XKB, the hook was forgotten and silently
dropped. The code calling this hook was removed by 7af53799c (4 Jan
2009), but it was probably already unused before that.

This patch re-adds support for this hook. The count hook parameter is
unused.

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---

I'm not intimately familiar with the xserver input code, but this seems
like a logical place to invoke this XACE hook. The various keyboard
devices call QueueKeyboardEvents, which places the events into a generic
input queue for sending to clients. So, I think this makes a decent
bottleneck through which we can expect all keyboard input events to
flow.

Updates to the XACE spec documentation for the count parameter will
follow, pending acceptance.


 dix/getevents.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/dix/getevents.c b/dix/getevents.c
index d0a87f7..e18248f 100644
--- a/dix/getevents.c
+++ b/dix/getevents.c
@@ -51,6 +51,7 @@
 #include inpututils.h
 #include mi.h
 #include windowstr.h
+#include xace.h
 
 #include X11/extensions/XKBproto.h
 #include xkbsrv.h
@@ -1058,9 +1059,24 @@ void
 QueueKeyboardEvents(DeviceIntPtr device, int type,
 int keycode)
 {
-int nevents;
+int nevents, i;
 
 nevents = GetKeyboardEvents(InputEventList, device, type, keycode);
+
+/* send KeyPress and KeyRelease events to XACE plugins */
+for (i = 0; i  nevents; i++) {
+if (InputEventList[i].any.type == ET_KeyPress ||
+InputEventList[i].any.type == ET_KeyRelease) {
+xEvent *core;
+int count;
+
+if (EventToCore(InputEventList[i], core, count) == Success) {
+XaceHook(XACE_KEY_AVAIL, core, device, 0);
+free(core);
+}
+}
+}
+
 queueEventList(device, InputEventList, nevents);
 }
 
-- 
2.3.5

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

Re: [PATCH] dix: Clear any existing selections before initializing privates

[Andrew Eikum]

On Fri, Feb 14, 2014 at 09:41:12AM -0600, Andrew Eikum wrote:
 On Mon, Feb 03, 2014 at 04:16:54PM -0500, Adam Jackson wrote:
  On Mon, 2013-12-30 at 09:15 -0600, Andrew Eikum wrote:
   To fix this, we should delete any existing selections before calling
   dixResetPrivates(). This will properly release the selection's
   privates and avoid the crash.
  
  Reviewed-by: Adam Jackson a...@redhat.com
  
 
 Thanks for the review, Adam. Is there anything else I can do to keep
 this patch moving?
 

Hi, we're less than a month out from the end of the 1.16 merge
window[1].

Any thoughts, updates, anything at all?

[1] http://lists.x.org/archives/xorg-devel/2013-December/039739.html

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

Re: [PATCH] dix: Clear any existing selections before initializing privates

[Andrew Eikum]

On Mon, Feb 03, 2014 at 04:16:54PM -0500, Adam Jackson wrote:
 On Mon, 2013-12-30 at 09:15 -0600, Andrew Eikum wrote:
  To fix this, we should delete any existing selections before calling
  dixResetPrivates(). This will properly release the selection's
  privates and avoid the crash.
 
 Reviewed-by: Adam Jackson a...@redhat.com
 

Thanks for the review, Adam. Is there anything else I can do to keep
this patch moving?

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

[PATCH] dix: Clear any existing selections before initializing privates

[Andrew Eikum]

If there is a selection left over from a previous execution of the
main loop, and that selection has privates allocated for it, the X
server will crash. This is because dixResetPrivates() resets the
privates refcounts to zero without accounting for the reference held
by the selection object. When the selection is then deleted in
InitSelections() after the call to dixResetPrivates(), the refcount
for its privates type goes negative and bad things happen.

To fix this, we should delete any existing selections before calling
dixResetPrivates(). This will properly release the selection's
privates and avoid the crash.

A more thorough description of the problem and a test case to
reproduce the crash is available at a previous mail:
  Negative Selection devPrivates refcount?
  By Andrew Eikum to xorg-devel on 10 Dec 2013
  http://lists.freedesktop.org/archives/xorg-devel/2013-December/039492.html

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---
 dix/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/dix/main.c b/dix/main.c
index 05dcbed..3a8af1a 100644
--- a/dix/main.c
+++ b/dix/main.c
@@ -175,6 +175,9 @@ dix_main(int argc, char *argv[], char *envp[])
 clients[0] = serverClient;
 currentMaxClients = 1;
 
+/* clear any existing selections */
+InitSelections();
+
 /* Initialize privates before first allocation */
 dixResetPrivates();
 
@@ -192,7 +195,6 @@ dix_main(int argc, char *argv[], char *envp[])
 
 InitAtoms();
 InitEvents();
-InitSelections();
 InitGlyphCaching();
 dixResetRegistry();
 ResetFontPrivateIndex();
-- 
1.8.5.2

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

Negative Selection devPrivates refcount?

[Andrew Eikum]

Hello,

I ran into an X server crash triggered by an X module I'm writing, and
eventually tracked the crash down to
'global_keys[PRIVATE_SELECTION].created' underflowing in
_dixFiniPrivates.

What seems to happen is:

1) The module registers a new private key with
dixRegisterPrivateKey(PRIVATE_XSELINUX).

2) Some X client (in my test case, fluxbox) creates a new Selection.
This Selection is allocated with _dixAllocateObjectWithPrivates, which
eventually increments global_keys[PRIVATE_SELECTION].created to 1.

3) I choose the 'Restart' option in fluxbox, which causes Dispatch()
in the main loop in dix/main.c to exit, and some teardown occurs.

4) On the next iteration of the loop, dixResetPrivates() is called,
which prints some warnings to the log file about various object
privates still being allocated. More importantly, it resets the
Selection privates 'created' variable to 0.

5) InitSelections() is then called, which iterates over all of the
existing Selections, including the one fluxbox created, in order to
delete them. As part of deleting them, it calls
dixFreeObjectWithPrivates(). This decrements that 'created' variable
to -1.

6) The module's init function re-registers its key with
dixRegisterPrivateKey. This triggers the assert
assert(!global_keys[t].created);. The X server crashes, as created
is -1.

Assuming my analysis is correct and I'm using the module and
devPrivates APIs correctly, this looks like an xserver bug. Probably
the best way to fix would be to clean up the Selections and such
before calling dixResetPrivates(). This would keep the log file clean,
and should resolve this issue by preventing the refcount from going
negative.  To do this, we'd just move the InitSelections() call to
earlier than the dixResetPrivates() call in the main loop. And, in
fact, this is enough to fix the crash for me.

What worries me about this approach is there are other objects left
allocated: 2 pixmaps, 4 GCs, and 1 cursor on my computer. Each of
these can have privates, and could potentially run into the same issue
depending on when they are cleaned up. Since it no longer crashes for
me after fixing the SELECTION issue, I guess it's not an issue, but
I'm not familiar enough with those parts of the code to be confident.

Another simple option for fixing would be refusing to go negative in
_dixFiniPrivates by checking if it's 0 before decrementing.

I've attached a simple X module to this mail which demonstrates the
problem on my computer with xserver-1.14.4. After building and
enabling the module in xorg.conf, I launch the X server, run fluxbox,
select Restart from the fluxbox menu, and the X server crashes.

Thoughts? Shall I go ahead and send such a patch after the 1.15
release in two weeks?

Thanks,
Andrew
#include xorg-server.h
#include xf86Module.h

static DevPrivateKeyRec key;

static XF86ModuleVersionInfo version = {
x,
x,
MODINFOSTRING1,
MODINFOSTRING2,
XORG_VERSION_CURRENT,
0,0,1,
ABI_CLASS_EXTENSION,
ABI_EXTENSION_VERSION,
MOD_CLASS_EXTENSION,
{0,0,0,0}
};

static void init(void)
{
memset(key, 0, sizeof(key));
if(!dixRegisterPrivateKey(key, PRIVATE_XSELINUX, 0))
xf86Msg(X_ERROR, Register key failed\n);
}

static pointer plug(pointer module, pointer options, int *errmaj, int *errmin)
{
ExtensionModule entry = {0};
entry.initFunc = init;
entry.name = devpriv;
LoadExtension(entry, FALSE);

return module;
}

_X_EXPORT XF86ModuleData devprivModuleData =
{
version,
plug,
NULL
};
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 1/3 xserver] xace: Add new hook to override Xtrans Read and Write invocations

[Andrew Eikum]

This hook is useful for environments where the security extension might
need to inject data into the stream or process out-of-band information
from clients. Between this and the following patch, I believe I have
added relevant hooks to all accept/read/write/close/disconnect calls in
the tree.

I only hook those five calls. I don't add hooks for other auxiliary
Xtrans calls such as XTransSetOption. The idea is that a security
extension would copy the existing Xtrans implementation and add
modifications. I didn't intend to allow security extensions to create an
entirely hot-pluggable Xtrans in a security extension.

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---

This is a resend of my three patches, rebased onto the latest
development branch. The first attempt[1] was over the holiday season,
so may have just been dropped on the floor.

Looking at the log for xserver:Xext/xace.c, it looks like XACE
doesn't see much active development these days, so perhaps there's no
one in particular to review these patches. I've CCd Eamon Walsh on the
series again, but otherwise what can I do to get these into the X.org
tree?

Thanks,
Andrew

[1]
http://lists.freedesktop.org/archives/xorg-devel/2012-December/034872.html

 Xext/xace.c | 17 -
 Xext/xace.h |  8 +++-
 Xext/xacestr.h  | 12 
 os/connection.c | 12 +---
 os/io.c | 30 +-
 5 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/Xext/xace.c b/Xext/xace.c
index 026d3c5..d51a720 100644
--- a/Xext/xace.c
+++ b/Xext/xace.c
@@ -103,12 +103,16 @@ XaceHook(int hook, ...)
 XaceScreenAccessRec screen;
 XaceAuthAvailRec auth;
 XaceKeyAvailRec key;
+XaceXtransRec xtrans;
 } u;
 int *prv = NULL;/* points to return value from callback */
 va_list ap; /* argument list */
 
-if (!XaceHooks[hook])
+if (!XaceHooks[hook]) {
+if (hook == XACE_XTRANS_DISPATCH)
+return BadImplementation;
 return Success;
+}
 
 va_start(ap, hook);
 
@@ -202,6 +206,17 @@ XaceHook(int hook, ...)
 u.key.count = va_arg(ap, int);
 
 break;
+case XACE_XTRANS_DISPATCH:
+u.xtrans.type = va_arg(ap, int);
+u.xtrans.ciptr = va_arg(ap, XtransConnInfo);
+u.xtrans.client = va_arg(ap, ClientPtr);
+u.xtrans.result = va_arg(ap, int *);
+u.xtrans.buf = va_arg(ap, char *);
+u.xtrans.size = va_arg(ap, int);
+
+u.xtrans.status = BadImplementation; /* signal no such hook */
+prv = u.xtrans.status;
+break;
 default:
 va_end(ap);
 return 0;   /* unimplemented hook number */
diff --git a/Xext/xace.h b/Xext/xace.h
index 5e6cb04..14f30e0 100644
--- a/Xext/xace.h
+++ b/Xext/xace.h
@@ -54,7 +54,13 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #define XACE_KEY_AVAIL			14
 #define XACE_AUDIT_BEGIN		15
 #define XACE_AUDIT_END			16
-#define XACE_NUM_HOOKS			17
+#define XACE_XTRANS_DISPATCH		17
+#define XACE_NUM_HOOKS			18
+
+#define XACE_XTRANS_READ 0
+#define XACE_XTRANS_WRITE 1
+#define XACE_XTRANS_READV 2
+#define XACE_XTRANS_WRITEV 3
 
 extern _X_EXPORT CallbackListPtr XaceHooks[XACE_NUM_HOOKS];
 
diff --git a/Xext/xacestr.h b/Xext/xacestr.h
index 989b033..144fead 100644
--- a/Xext/xacestr.h
+++ b/Xext/xacestr.h
@@ -28,6 +28,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include property.h
 #include selection.h
 #include xace.h
+#include X11/Xtrans/Xtrans.h
 
 /* XACE_CORE_DISPATCH */
 typedef struct {
@@ -144,4 +145,15 @@ typedef struct {
 int requestResult;
 } XaceAuditRec;
 
+/* XACE_XTRANS_DISPATCH */
+typedef struct {
+int type; /* one of XACE_XTRANS_* */
+XtransConnInfo ciptr;
+ClientPtr client;
+int *result;
+char *buf;
+int size;
+int status;
+} XaceXtransRec;
+
 #endif  /* _XACESTR_H */
diff --git a/os/connection.c b/os/connection.c
index 6cd8bcf..6ddb9aa 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -890,7 +890,7 @@ return TRUE;
 static void
 ErrorConnMax(XtransConnInfo trans_conn)
 {
-int fd = _XSERVTransGetConnectionNumber(trans_conn);
+int fd = _XSERVTransGetConnectionNumber(trans_conn), err, result;
 xConnSetupPrefix csp;
 char pad[3] = { 0, 0, 0 };
 struct iovec iov[3];
@@ -907,7 +907,10 @@ ErrorConnMax(XtransConnInfo trans_conn)
 FD_SET(fd, mask);
 (void) Select(fd + 1, mask, NULL, NULL, waittime);
 /* try to read the byte-order of the connection */
-(void) _XSERVTransRead(trans_conn, order, 1);
+err = XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_READ, trans_conn,
+NULL, result, order, 1);
+if (err == BadImplementation)
+(void) _XSERVTransRead(trans_conn, order, 1);
 if (order == 'l' || order == 'B' || order == 'r' || order == 'R') {
 csp.success = xFalse

[PATCH 2/3 xserver] xace: Add XACE hooks for Xtrans Accept/Disconnect/Close

[Andrew Eikum]

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---
 Xext/xace.c |  2 +-
 Xext/xace.h | 11 +++
 Xext/xacestr.h  |  2 +-
 os/connection.c | 40 ++--
 os/io.c | 16 
 os/osdep.h  |  3 +++
 6 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/Xext/xace.c b/Xext/xace.c
index d51a720..2394024 100644
--- a/Xext/xace.c
+++ b/Xext/xace.c
@@ -208,7 +208,7 @@ XaceHook(int hook, ...)
 break;
 case XACE_XTRANS_DISPATCH:
 u.xtrans.type = va_arg(ap, int);
-u.xtrans.ciptr = va_arg(ap, XtransConnInfo);
+u.xtrans.pciptr = va_arg(ap, XtransConnInfo *);
 u.xtrans.client = va_arg(ap, ClientPtr);
 u.xtrans.result = va_arg(ap, int *);
 u.xtrans.buf = va_arg(ap, char *);
diff --git a/Xext/xace.h b/Xext/xace.h
index 14f30e0..91c5815 100644
--- a/Xext/xace.h
+++ b/Xext/xace.h
@@ -57,10 +57,13 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #define XACE_XTRANS_DISPATCH		17
 #define XACE_NUM_HOOKS			18
 
-#define XACE_XTRANS_READ 0
-#define XACE_XTRANS_WRITE 1
-#define XACE_XTRANS_READV 2
-#define XACE_XTRANS_WRITEV 3
+#define XACE_XTRANS_ACCEPT 0
+#define XACE_XTRANS_DISCONNECT 1
+#define XACE_XTRANS_CLOSE 2
+#define XACE_XTRANS_READ 3
+#define XACE_XTRANS_WRITE 4
+#define XACE_XTRANS_READV 5
+#define XACE_XTRANS_WRITEV 6
 
 extern _X_EXPORT CallbackListPtr XaceHooks[XACE_NUM_HOOKS];
 
diff --git a/Xext/xacestr.h b/Xext/xacestr.h
index 144fead..dee49a8 100644
--- a/Xext/xacestr.h
+++ b/Xext/xacestr.h
@@ -148,7 +148,7 @@ typedef struct {
 /* XACE_XTRANS_DISPATCH */
 typedef struct {
 int type; /* one of XACE_XTRANS_* */
-XtransConnInfo ciptr;
+XtransConnInfo *pciptr;
 ClientPtr client;
 int *result;
 char *buf;
diff --git a/os/connection.c b/os/connection.c
index 6ddb9aa..6688d14 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -507,12 +507,28 @@ ResetWellKnownSockets(void)
 }
 
 void
+CallXtransClose(ClientPtr client, XtransConnInfo *pciptr)
+{
+if (XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_CLOSE, pciptr,
+client, NULL, NULL, 0) == BadImplementation)
+_XSERVTransClose(*pciptr);
+}
+
+void
+CallXtransDisconnect(ClientPtr client, XtransConnInfo *pciptr)
+{
+if (XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_DISCONNECT, pciptr,
+client, NULL, NULL, 0) == BadImplementation)
+_XSERVTransDisconnect(*pciptr);
+}
+
+void
 CloseWellKnownConnections(void)
 {
 int i;
 
 for (i = 0; i  ListenTransCount; i++)
-_XSERVTransClose(ListenTransConns[i]);
+CallXtransClose(serverClient, ListenTransConns[i]);
 }
 
 static void
@@ -805,7 +821,7 @@ EstablishNewConnections(ClientPtr clientUnused, pointer closure)
 register int newconn;   /* fd of new client */
 CARD32 connect_time;
 register int i;
-register ClientPtr client;
+register ClientPtr client = NULL;
 register OsCommPtr oc;
 fd_set tmask;
 
@@ -845,7 +861,11 @@ EstablishNewConnections(ClientPtr clientUnused, pointer closure)
 if ((trans_conn = lookup_trans_conn(curconn)) == NULL)
 continue;
 
-if ((new_trans_conn = _XSERVTransAccept(trans_conn, status)) == NULL)
+new_trans_conn = trans_conn;
+if (XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_ACCEPT, new_trans_conn,
+NULL, status, NULL, 0) == BadImplementation)
+new_trans_conn = _XSERVTransAccept(trans_conn, status);
+if (new_trans_conn == trans_conn || !new_trans_conn)
 continue;
 
 newconn = _XSERVTransGetConnectionNumber(new_trans_conn);
@@ -869,7 +889,7 @@ EstablishNewConnections(ClientPtr clientUnused, pointer closure)
 
 if (!AllocNewConnection(new_trans_conn, newconn, connect_time)) {
 ErrorConnMax(new_trans_conn);
-_XSERVTransClose(new_trans_conn);
+CallXtransClose(client, new_trans_conn);
 }
 }
 #ifndef WIN32
@@ -907,7 +927,7 @@ ErrorConnMax(XtransConnInfo trans_conn)
 FD_SET(fd, mask);
 (void) Select(fd + 1, mask, NULL, NULL, waittime);
 /* try to read the byte-order of the connection */
-err = XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_READ, trans_conn,
+err = XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_READ, trans_conn,
 NULL, result, order, 1);
 if (err == BadImplementation)
 (void) _XSERVTransRead(trans_conn, order, 1);
@@ -929,7 +949,7 @@ ErrorConnMax(XtransConnInfo trans_conn)
 iov[1].iov_base = NOROOM;
 iov[2].iov_len = (4 - (csp.lengthReason  3))  3;
 iov[2].iov_base = pad;
-err = XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_WRITEV, trans_conn,
+err = XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_WRITEV, trans_conn,
 NULL, result, iov, 3);
 if (err == BadImplementation)
 (void) _XSERVTransWritev(trans_conn, iov, 3);
@@ -942,13 +962,13 @@ ErrorConnMax

[PATCH 3/3 xorg-docs] Add documentation for the new Xtrans Dispatch Hook

[Andrew Eikum]

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---
 specs/Xserver/XACE-Spec.xml | 64 +
 1 file changed, 64 insertions(+)

diff --git a/specs/Xserver/XACE-Spec.xml b/specs/Xserver/XACE-Spec.xml
index bc7340d..1a5bb51 100644
--- a/specs/Xserver/XACE-Spec.xml
+++ b/specs/Xserver/XACE-Spec.xml
@@ -310,6 +310,11 @@
 		entrytypeXaceAuditRec/type/entry
 		entryxref linkend=audit_avail_hook//entry
 	  /row
+	  row
+		entryliteralXACE_XTRANS_DISPATCH/literal/entry
+		entrytypeXaceXtransRec/type/entry
+		entryxref linkend=xtrans_dispatch_hook//entry
+	  /row
 	/tbody
 	  /tgroup
 	/table
@@ -1044,6 +1049,65 @@
 	  paraThese hooks have no return value./para
 	/section
 
+	section id=xtrans_dispatch_hook
+	  titleXtrans Dispatch Hook/title
+	  paraThis hook allows a security extension to override the Xtrans methods normally used for communicating between the server and client processes. The hook argument is
+	  a pointer to a structure of type typeXaceXtransRec/type. This structure contains a structfieldtype/structfield field of type typeint/type, a structfieldpciptr/structfield
+	  field of type typeXtransConnInfo*/type, a structfieldclient/structfield field of type typeClientPtr/type, a structfieldresult/structfield field of type
+	  typeint*/type, a structfieldbuf/structfield field of type typechar*/type, a structfieldsize/structfield field of type typeint/type, and a
+	  structfieldstatus/structfield field of type typeint/type./para
+	  paraThe structfieldtype/structfield field provides the type of Xtrans transaction being performed. It may be one of the following values./para
+	  table frame=all id=xtrans_dispatch_types
+	titleXtrans Dispatch Types/title
+	tgroup cols='2' align='left' colsep='1' rowsep='1'
+	  thead
+		row
+		  entryType Flag/entry
+		  entryNotes/entry
+		/row
+	  /thead
+	  tbody
+		row
+		  entryliteralXACE_XTRANS_ACCEPT/literal/entry
+		  entryA client is attempting to connect to the server. This is analogous to the Xtrans Accept function. It is expected that the extension will fill structfield*pciptr/structfield with a newly allocated and initialized typestruct _XtransConnInfo/type if the extension handles this request type./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_READ/literal/entry
+		  entryThe server is requesting a read on the connection. This is analogous to the Xtrans Read function./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_READV/literal/entry
+		  entryThe server is requesting a read on the connection. The structfieldbuf/structfield field is of type typeiovec*/type and the structfieldsize/structfield field contains the number of typeiovec/types. This is analogous to the Xtrans Readv function./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_WRITE/literal/entry
+		  entryThe server is requesting a write on the connection. This is analogous to the Xtrans Write function./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_WRITEV/literal/entry
+		  entryThe server is requesting a write on the connection. The structfieldbuf/structfield field is of type typeiovec*/type and the structfieldsize/structfield field contains the number of typeiovec/types. This is analogous to the Xtrans Writev function./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_CLOSE/literal/entry
+		  entryA client's connection with the server is being closed. This is analogous to the Xtrans Close function. The extension should clear resources allocated during the matching literalXACE_XTRANS_ACCEPT/literal request, including the contents of structfield*pciptr/structfield./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_DISCONNECT/literal/entry
+		  entryA client's connection with the server is being shut down. This is analogous to the Xtrans Disconnect function./entry
+		/row
+	  /tbody
+	/tgroup
+	  /table
+	  paraThe structfieldpciptr/structfield field provides the typeXtransConnInfo/type for the connection. One level of redirection is required to use. See the literalXACE_XTRANS_ACCEPT/literal type description, above./para
+	  paraThe structfieldclient/structfield field provides the typeClientPtr/type for the connection./para
+	  paraThe structfieldresult/structfield field should be used to return the appropriate result for the matching Xtrans function. For example, an literalXACE_XTRANS_READ/literal request should return the number of bytes read./para
+	  paraThe structfieldbuf/structfield field is where data is read from or written into, depending on the request type. May be typechar*/type or typeiovec*/type depending on the request type (see table above)./para
+	  paraThe structfieldsize/structfield field contains the size of the structfieldbuf/structfield field, in either byte count or typeiovec/type count depending on the request type (see table above)./para
+	  paraThe structfieldstatus/structfield field may be set to a nonzero X protocol error code. If structfieldstatus/structfield

[PATCH 2/3 xserver] xace: Add XACE hooks for Xtrans Accept/Disconnect/Close

[Andrew Eikum]

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---
 Xext/xace.c |  2 +-
 Xext/xace.h | 11 +++
 Xext/xacestr.h  |  2 +-
 os/connection.c | 40 ++--
 os/io.c | 16 
 os/osdep.h  |  3 +++
 6 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/Xext/xace.c b/Xext/xace.c
index d51a720..2394024 100644
--- a/Xext/xace.c
+++ b/Xext/xace.c
@@ -208,7 +208,7 @@ XaceHook(int hook, ...)
 break;
 case XACE_XTRANS_DISPATCH:
 u.xtrans.type = va_arg(ap, int);
-u.xtrans.ciptr = va_arg(ap, XtransConnInfo);
+u.xtrans.pciptr = va_arg(ap, XtransConnInfo *);
 u.xtrans.client = va_arg(ap, ClientPtr);
 u.xtrans.result = va_arg(ap, int *);
 u.xtrans.buf = va_arg(ap, char *);
diff --git a/Xext/xace.h b/Xext/xace.h
index 14f30e0..91c5815 100644
--- a/Xext/xace.h
+++ b/Xext/xace.h
@@ -57,10 +57,13 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #define XACE_XTRANS_DISPATCH		17
 #define XACE_NUM_HOOKS			18
 
-#define XACE_XTRANS_READ 0
-#define XACE_XTRANS_WRITE 1
-#define XACE_XTRANS_READV 2
-#define XACE_XTRANS_WRITEV 3
+#define XACE_XTRANS_ACCEPT 0
+#define XACE_XTRANS_DISCONNECT 1
+#define XACE_XTRANS_CLOSE 2
+#define XACE_XTRANS_READ 3
+#define XACE_XTRANS_WRITE 4
+#define XACE_XTRANS_READV 5
+#define XACE_XTRANS_WRITEV 6
 
 extern _X_EXPORT CallbackListPtr XaceHooks[XACE_NUM_HOOKS];
 
diff --git a/Xext/xacestr.h b/Xext/xacestr.h
index 144fead..dee49a8 100644
--- a/Xext/xacestr.h
+++ b/Xext/xacestr.h
@@ -148,7 +148,7 @@ typedef struct {
 /* XACE_XTRANS_DISPATCH */
 typedef struct {
 int type; /* one of XACE_XTRANS_* */
-XtransConnInfo ciptr;
+XtransConnInfo *pciptr;
 ClientPtr client;
 int *result;
 char *buf;
diff --git a/os/connection.c b/os/connection.c
index 6ddb9aa..6688d14 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -507,12 +507,28 @@ ResetWellKnownSockets(void)
 }
 
 void
+CallXtransClose(ClientPtr client, XtransConnInfo *pciptr)
+{
+if (XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_CLOSE, pciptr,
+client, NULL, NULL, 0) == BadImplementation)
+_XSERVTransClose(*pciptr);
+}
+
+void
+CallXtransDisconnect(ClientPtr client, XtransConnInfo *pciptr)
+{
+if (XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_DISCONNECT, pciptr,
+client, NULL, NULL, 0) == BadImplementation)
+_XSERVTransDisconnect(*pciptr);
+}
+
+void
 CloseWellKnownConnections(void)
 {
 int i;
 
 for (i = 0; i  ListenTransCount; i++)
-_XSERVTransClose(ListenTransConns[i]);
+CallXtransClose(serverClient, ListenTransConns[i]);
 }
 
 static void
@@ -805,7 +821,7 @@ EstablishNewConnections(ClientPtr clientUnused, pointer closure)
 register int newconn;   /* fd of new client */
 CARD32 connect_time;
 register int i;
-register ClientPtr client;
+register ClientPtr client = NULL;
 register OsCommPtr oc;
 fd_set tmask;
 
@@ -845,7 +861,11 @@ EstablishNewConnections(ClientPtr clientUnused, pointer closure)
 if ((trans_conn = lookup_trans_conn(curconn)) == NULL)
 continue;
 
-if ((new_trans_conn = _XSERVTransAccept(trans_conn, status)) == NULL)
+new_trans_conn = trans_conn;
+if (XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_ACCEPT, new_trans_conn,
+NULL, status, NULL, 0) == BadImplementation)
+new_trans_conn = _XSERVTransAccept(trans_conn, status);
+if (new_trans_conn == trans_conn || !new_trans_conn)
 continue;
 
 newconn = _XSERVTransGetConnectionNumber(new_trans_conn);
@@ -869,7 +889,7 @@ EstablishNewConnections(ClientPtr clientUnused, pointer closure)
 
 if (!AllocNewConnection(new_trans_conn, newconn, connect_time)) {
 ErrorConnMax(new_trans_conn);
-_XSERVTransClose(new_trans_conn);
+CallXtransClose(client, new_trans_conn);
 }
 }
 #ifndef WIN32
@@ -907,7 +927,7 @@ ErrorConnMax(XtransConnInfo trans_conn)
 FD_SET(fd, mask);
 (void) Select(fd + 1, mask, NULL, NULL, waittime);
 /* try to read the byte-order of the connection */
-err = XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_READ, trans_conn,
+err = XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_READ, trans_conn,
 NULL, result, order, 1);
 if (err == BadImplementation)
 (void) _XSERVTransRead(trans_conn, order, 1);
@@ -929,7 +949,7 @@ ErrorConnMax(XtransConnInfo trans_conn)
 iov[1].iov_base = NOROOM;
 iov[2].iov_len = (4 - (csp.lengthReason  3))  3;
 iov[2].iov_base = pad;
-err = XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_WRITEV, trans_conn,
+err = XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_WRITEV, trans_conn,
 NULL, result, iov, 3);
 if (err == BadImplementation)
 (void) _XSERVTransWritev(trans_conn, iov, 3);
@@ -942,13 +962,13 @@ ErrorConnMax

[PATCH 1/3 xserver] xace: Add new hook to override Xtrans Read and Write invocations

[Andrew Eikum]

This hook is useful for environments where the security extension might
need to inject data into the stream or process out-of-band information
from clients. Between this and the following patch, I believe I have
added relevant hooks to all accept/read/write/close/disconnect calls in
the tree.

I only hook those five calls. I don't add hooks for other auxiliary
Xtrans calls such as XTransSetOption. The idea is that a security
extension would copy the existing Xtrans implementation and add
modifications. I didn't intend to allow security extensions to create an
entirely hot-pluggable Xtrans in a security extension.

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---

As always with a new project, it's hard to be sure if I'm following
all of the submission rules :)

Looking at the calendar, I think I missed the 1.13 merge window.
Hopefully we can get any problems worked out and target 1.14 with this
series, then.

I appreciate any feedback!

Thanks,
Andrew

 Xext/xace.c | 17 -
 Xext/xace.h |  8 +++-
 Xext/xacestr.h  | 12 
 os/connection.c | 12 +---
 os/io.c | 30 +-
 5 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/Xext/xace.c b/Xext/xace.c
index 026d3c5..d51a720 100644
--- a/Xext/xace.c
+++ b/Xext/xace.c
@@ -103,12 +103,16 @@ XaceHook(int hook, ...)
 XaceScreenAccessRec screen;
 XaceAuthAvailRec auth;
 XaceKeyAvailRec key;
+XaceXtransRec xtrans;
 } u;
 int *prv = NULL;/* points to return value from callback */
 va_list ap; /* argument list */
 
-if (!XaceHooks[hook])
+if (!XaceHooks[hook]) {
+if (hook == XACE_XTRANS_DISPATCH)
+return BadImplementation;
 return Success;
+}
 
 va_start(ap, hook);
 
@@ -202,6 +206,17 @@ XaceHook(int hook, ...)
 u.key.count = va_arg(ap, int);
 
 break;
+case XACE_XTRANS_DISPATCH:
+u.xtrans.type = va_arg(ap, int);
+u.xtrans.ciptr = va_arg(ap, XtransConnInfo);
+u.xtrans.client = va_arg(ap, ClientPtr);
+u.xtrans.result = va_arg(ap, int *);
+u.xtrans.buf = va_arg(ap, char *);
+u.xtrans.size = va_arg(ap, int);
+
+u.xtrans.status = BadImplementation; /* signal no such hook */
+prv = u.xtrans.status;
+break;
 default:
 va_end(ap);
 return 0;   /* unimplemented hook number */
diff --git a/Xext/xace.h b/Xext/xace.h
index 5e6cb04..14f30e0 100644
--- a/Xext/xace.h
+++ b/Xext/xace.h
@@ -54,7 +54,13 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #define XACE_KEY_AVAIL			14
 #define XACE_AUDIT_BEGIN		15
 #define XACE_AUDIT_END			16
-#define XACE_NUM_HOOKS			17
+#define XACE_XTRANS_DISPATCH		17
+#define XACE_NUM_HOOKS			18
+
+#define XACE_XTRANS_READ 0
+#define XACE_XTRANS_WRITE 1
+#define XACE_XTRANS_READV 2
+#define XACE_XTRANS_WRITEV 3
 
 extern _X_EXPORT CallbackListPtr XaceHooks[XACE_NUM_HOOKS];
 
diff --git a/Xext/xacestr.h b/Xext/xacestr.h
index 989b033..144fead 100644
--- a/Xext/xacestr.h
+++ b/Xext/xacestr.h
@@ -28,6 +28,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include property.h
 #include selection.h
 #include xace.h
+#include X11/Xtrans/Xtrans.h
 
 /* XACE_CORE_DISPATCH */
 typedef struct {
@@ -144,4 +145,15 @@ typedef struct {
 int requestResult;
 } XaceAuditRec;
 
+/* XACE_XTRANS_DISPATCH */
+typedef struct {
+int type; /* one of XACE_XTRANS_* */
+XtransConnInfo ciptr;
+ClientPtr client;
+int *result;
+char *buf;
+int size;
+int status;
+} XaceXtransRec;
+
 #endif  /* _XACESTR_H */
diff --git a/os/connection.c b/os/connection.c
index 6cd8bcf..6ddb9aa 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -890,7 +890,7 @@ return TRUE;
 static void
 ErrorConnMax(XtransConnInfo trans_conn)
 {
-int fd = _XSERVTransGetConnectionNumber(trans_conn);
+int fd = _XSERVTransGetConnectionNumber(trans_conn), err, result;
 xConnSetupPrefix csp;
 char pad[3] = { 0, 0, 0 };
 struct iovec iov[3];
@@ -907,7 +907,10 @@ ErrorConnMax(XtransConnInfo trans_conn)
 FD_SET(fd, mask);
 (void) Select(fd + 1, mask, NULL, NULL, waittime);
 /* try to read the byte-order of the connection */
-(void) _XSERVTransRead(trans_conn, order, 1);
+err = XaceHook(XACE_XTRANS_DISPATCH, XACE_XTRANS_READ, trans_conn,
+NULL, result, order, 1);
+if (err == BadImplementation)
+(void) _XSERVTransRead(trans_conn, order, 1);
 if (order == 'l' || order == 'B' || order == 'r' || order == 'R') {
 csp.success = xFalse;
 csp.lengthReason = sizeof(NOROOM) - 1;
@@ -926,7 +929,10 @@ ErrorConnMax(XtransConnInfo trans_conn)
 iov[1].iov_base = NOROOM;
 iov[2].iov_len = (4 - (csp.lengthReason  3))  3;
 iov[2].iov_base = pad;
-(void) _XSERVTransWritev(trans_conn

[PATCH 3/3 xorg-docs] Add documentation for the new Xtrans Dispatch Hook

[Andrew Eikum]

Signed-off-by: Andrew Eikum aei...@codeweavers.com
---
 specs/Xserver/XACE-Spec.xml | 64 +
 1 file changed, 64 insertions(+)

diff --git a/specs/Xserver/XACE-Spec.xml b/specs/Xserver/XACE-Spec.xml
index bc7340d..1a5bb51 100644
--- a/specs/Xserver/XACE-Spec.xml
+++ b/specs/Xserver/XACE-Spec.xml
@@ -310,6 +310,11 @@
 		entrytypeXaceAuditRec/type/entry
 		entryxref linkend=audit_avail_hook//entry
 	  /row
+	  row
+		entryliteralXACE_XTRANS_DISPATCH/literal/entry
+		entrytypeXaceXtransRec/type/entry
+		entryxref linkend=xtrans_dispatch_hook//entry
+	  /row
 	/tbody
 	  /tgroup
 	/table
@@ -1044,6 +1049,65 @@
 	  paraThese hooks have no return value./para
 	/section
 
+	section id=xtrans_dispatch_hook
+	  titleXtrans Dispatch Hook/title
+	  paraThis hook allows a security extension to override the Xtrans methods normally used for communicating between the server and client processes. The hook argument is
+	  a pointer to a structure of type typeXaceXtransRec/type. This structure contains a structfieldtype/structfield field of type typeint/type, a structfieldpciptr/structfield
+	  field of type typeXtransConnInfo*/type, a structfieldclient/structfield field of type typeClientPtr/type, a structfieldresult/structfield field of type
+	  typeint*/type, a structfieldbuf/structfield field of type typechar*/type, a structfieldsize/structfield field of type typeint/type, and a
+	  structfieldstatus/structfield field of type typeint/type./para
+	  paraThe structfieldtype/structfield field provides the type of Xtrans transaction being performed. It may be one of the following values./para
+	  table frame=all id=xtrans_dispatch_types
+	titleXtrans Dispatch Types/title
+	tgroup cols='2' align='left' colsep='1' rowsep='1'
+	  thead
+		row
+		  entryType Flag/entry
+		  entryNotes/entry
+		/row
+	  /thead
+	  tbody
+		row
+		  entryliteralXACE_XTRANS_ACCEPT/literal/entry
+		  entryA client is attempting to connect to the server. This is analogous to the Xtrans Accept function. It is expected that the extension will fill structfield*pciptr/structfield with a newly allocated and initialized typestruct _XtransConnInfo/type if the extension handles this request type./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_READ/literal/entry
+		  entryThe server is requesting a read on the connection. This is analogous to the Xtrans Read function./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_READV/literal/entry
+		  entryThe server is requesting a read on the connection. The structfieldbuf/structfield field is of type typeiovec*/type and the structfieldsize/structfield field contains the number of typeiovec/types. This is analogous to the Xtrans Readv function./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_WRITE/literal/entry
+		  entryThe server is requesting a write on the connection. This is analogous to the Xtrans Write function./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_WRITEV/literal/entry
+		  entryThe server is requesting a write on the connection. The structfieldbuf/structfield field is of type typeiovec*/type and the structfieldsize/structfield field contains the number of typeiovec/types. This is analogous to the Xtrans Writev function./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_CLOSE/literal/entry
+		  entryA client's connection with the server is being closed. This is analogous to the Xtrans Close function. The extension should clear resources allocated during the matching literalXACE_XTRANS_ACCEPT/literal request, including the contents of structfield*pciptr/structfield./entry
+		/row
+		row
+		  entryliteralXACE_XTRANS_DISCONNECT/literal/entry
+		  entryA client's connection with the server is being shut down. This is analogous to the Xtrans Disconnect function./entry
+		/row
+	  /tbody
+	/tgroup
+	  /table
+	  paraThe structfieldpciptr/structfield field provides the typeXtransConnInfo/type for the connection. One level of redirection is required to use. See the literalXACE_XTRANS_ACCEPT/literal type description, above./para
+	  paraThe structfieldclient/structfield field provides the typeClientPtr/type for the connection./para
+	  paraThe structfieldresult/structfield field should be used to return the appropriate result for the matching Xtrans function. For example, an literalXACE_XTRANS_READ/literal request should return the number of bytes read./para
+	  paraThe structfieldbuf/structfield field is where data is read from or written into, depending on the request type. May be typechar*/type or typeiovec*/type depending on the request type (see table above)./para
+	  paraThe structfieldsize/structfield field contains the size of the structfieldbuf/structfield field, in either byte count or typeiovec/type count depending on the request type (see table above)./para
+	  paraThe structfieldstatus/structfield field may be set to a nonzero X protocol error code. If structfieldstatus/structfield