Bug 50641

Summary: xorg-server-1.12.0 - When SELinux is enabled the xserver fails
Product: xorg Reporter: Richard Haines <rch_bugs>
Component: Server/GeneralAssignee: Xorg Project Team <xorg-team>
Status: RESOLVED MOVED QA Contact: Xorg Project Team <xorg-team>
Severity: major    
Priority: high CC: ewalsh, jeremyhu, peter.hutterer
Version: unspecifiedKeywords: patch, regression
Hardware: Other   
OS: Linux (All)   
Whiteboard: 2012BRB_Reviewed
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 40982    

Description Richard Haines 2012-06-03 08:05:55 UTC
Using Fedora 17 with xorg-server-1.12.0 on Dell (x86_64)

The xorg-server-1.11.4 works fine with Fedora 16

When using targeted policy:
1) setsebool xserver_object_manager on
2) init 3
3) logon as root
4) init 5
5) X does not load. 

This always happens at the same point with the Xorg log giving some infos but I managed to get the following core dump that has more detail:


(gdb) info stack
#0  0x0000003f94635965 in __GI_raise (sig=sig@entry=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x0000003f94637118 in __GI_abort () at abort.c:91
#2  0x000000000046c41e in OsAbort () at utils.c:1249
#3  0x000000000048278c in ddxGiveUp (error=EXIT_ERR_ABORT) at xf86Init.c:989
#4  0x00000000004689f2 in AbortServer () at log.c:480
#5  0x0000000000468bf5 in FatalError (
    f=f@entry=0x571f58 "Caught signal %d (%s). Server aborting\n") at log.c:611
#6  0x0000000000469e8e in OsSigHandler (sip=<optimized out>, signo=11, 
    unused=<optimized out>) at osinit.c:146
#7  OsSigHandler (signo=11, sip=<optimized out>, unused=<optimized out>)
    at osinit.c:108
#8  <signal handler called>
#9  0x00007f9d6708e77a in SELinuxDoCheck (auditdata=<optimized out>, 
    mode=131072, class=11, subj=0x2af3670, obj=<optimized out>)
    at xselinux_hooks.c:98
#10 SELinuxDoCheck (subj=0x2af3670, class=11, mode=131072, 
    auditdata=<optimized out>, 
    obj=<error reading variable: Unhandled dwarf expression opcode 0xfa>)
    at xselinux_hooks.c:88
#11 0x00007f9d6708f630 in SELinuxDevice (pcbl=<optimized out>, 
    unused=<optimized out>, calldata=0x2af3670) at xselinux_hooks.c:364
#12 0x0000000000438ad4 in _CallCallbacks (pcbl=0x2a64730, call_data=0x7d6718, 
---Type <return> to continue, or q <return> to quit--- 
    call_data@entry=0x7fff92c52930) at dixutils.c:752
#13 0x00000000004e3018 in CallCallbacks (call_data=0x7fff92c52930, 
    pcbl=<optimized out>) at ../include/callback.h:86
#14 XaceHook (hook=hook@entry=3) at xace.c:196
#15 0x0000000000507690 in GrabKey (client=client@entry=0x2af35b0, 
    dev=0x7d6ce0, modifier_device=modifier_device@entry=0x7d6ce0, 
    key=-1832572624, param=param@entry=0x7fff92c52a90, 
    grabtype=grabtype@entry=XI2, mask=mask@entry=0x7fff92c52a50)
    at exevents.c:2389
#16 0x000000000051138c in ProcXIPassiveGrabDevice (client=0x2af35b0)
    at xipassivegrab.c:205
#17 0x000000000043439a in Dispatch () at dispatch.c:439
#18 0x00000000004233e5 in main (argc=13, argv=0x7fff92c52ca8, 
    envp=<optimized out>) at main.c:287
(gdb)
Comment 1 Richard Haines 2012-06-07 10:06:49 UTC
This patch will fix the problem but: Should the keyboard be initialised by the 
SELinuxLabelInitial(); routine in Xext/xselinux_hooks.c so that it has a
context. I could not find a way to do this but as device IDs 0 and 1 are
reserved, then this patch seems reasonable!!!


When SELinux is enabled by 'setsebool xserver_object_manager on' the xserver
will not load. This is caused when ProcXIPassiveUngrabDevice
(in Xi/xipassivegrab.c) calls GrabKey (in Xi/exevents.c) with a device ID
of '1'. This is the keyboard that does not have a security context attached to
it.

Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
 Xi/exevents.c |    9 +++++----
 dix/grabs.c   |    8 +++++---
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/Xi/exevents.c b/Xi/exevents.c
index f390f67..a8f9cb8 100644
--- a/Xi/exevents.c
+++ b/Xi/exevents.c
@@ -2386,10 +2386,11 @@ GrabKey(ClientPtr client, DeviceIntPtr dev, DeviceIntPtr modifier_device,
 	return rc;
     if (param->this_device_mode == GrabModeSync || param->other_devices_mode == GrabModeSync)
 	access_mode |= DixFreezeAccess;
-    rc = XaceHook(XACE_DEVICE_ACCESS, client, dev, access_mode);
-    if (rc != Success)
-	return rc;
-
+	if (dev->id > 1) {
+	    rc = XaceHook(XACE_DEVICE_ACCESS, client, dev, access_mode);
+    	if (rc != Success)
+			return rc;
+	}
     grab = CreateGrab(client->index, dev, modifier_device, pWin, grabtype,
                       mask, param, type, key, NULL, NULL);
     if (!grab)
diff --git a/dix/grabs.c b/dix/grabs.c
index cc2c946..cc64515 100644
--- a/dix/grabs.c
+++ b/dix/grabs.c
@@ -583,9 +583,11 @@ AddPassiveGrabToList(ClientPtr client, GrabPtr pGrab)
 
     if (pGrab->keyboardMode == GrabModeSync||pGrab->pointerMode == GrabModeSync)
 	access_mode |= DixFreezeAccess;
-    rc = XaceHook(XACE_DEVICE_ACCESS, client, pGrab->device, access_mode);
-    if (rc != Success)
-	return rc;
+	if (pGrab->device->id > 1) {
+	    rc = XaceHook(XACE_DEVICE_ACCESS, client, pGrab->device, access_mode);
+    	if (rc != Success)
+			return rc;
+	}
 
     /* Remove all grabs that match the new one exactly */
     for (grab = wPassiveGrabs(pGrab->window); grab; grab = grab->next)
-- 
1.7.10.2
Comment 2 Richard Haines 2012-06-08 08:21:16 UTC
I've been trying to find a better fix but but could only come up with the one
below. Anyway I'm coming to the conclusion that maybe ProcXIPassiveUngrabDevice
should not be called with device id=1 as 0 and 1 are reserved (according to
the comments in dix/device.c)

I have not managed to find why device id=1 is used or where the call originates
from but ProcXIPassiveUngrabDevice is called 91 times with the same 
parameters before X is loaded. Does anyone have any info that may help.

The simplest fix to get X working with SELinux is to modify the 
Xi/xipassivegrab.c ProcXIPassiveGrabDevice() function:

---
 Xi/xipassivegrab.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/Xi/xipassivegrab.c b/Xi/xipassivegrab.c
index c80da80..b7e8326 100644
--- a/Xi/xipassivegrab.c
+++ b/Xi/xipassivegrab.c
@@ -192,6 +192,8 @@ ProcXIPassiveGrabDevice(ClientPtr client)
 
     for (i = 0; i < stuff->num_modifiers; i++, modifiers++)
     {
+		if (dev->id < 2)
+			continue;
         uint8_t status = Success;
 
         param.modifiers = *modifiers;
-- 
1.7.10.2
Comment 3 Richard Haines 2012-06-12 06:54:54 UTC
The final fix (well mine anyway):

This patch was created using xorg-server-1.12.0 source.

The bug is caused by X calling XaceHook(XACE_DEVICE_ACCESS, client, ...)
with a device ID of '1' that is XIAllMasterDevices. It would also happen
if the device ID = 0 (XIAllDevices).

The only places currently seen calling with a device id=1 are:
GrabKey - in Xi/exevents.c and AddPassiveGrabToList - in dix/grabs.c
These start life in ProcXIPassiveGrabDevice (in Xi/xipassivegrab.c) that
has been called by XIGrabKeycode.

The patch has been tested using the other XI calls that would also impact
this: XIGrabTouchBegin, XIGrabButton, XIGrabFocusIn and XIGrabEnter with
and without the correct permissions (grab and freeze) with no problems.

Both possible classes have to be checked (x_keyboard and x_pointer) as it
is not known whether it is a pointer or keyboard as this info is not
available. To get this info would require a change to the
XaceHook(XACE_DEVICE_ACCESS, client, ..) call to pass an additional
parameter stating the actual devices (that would defeat the objective of
the XIAllMasterDevices and XIAllDevices dev ids).

Note that there are other devices apart from the keyboard and pointer, for
example on the test system: DeviceID: 9 is the Integrated_Webcam_1.3M. As
it is classed as a slave keyboard it is checked.

Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
Xext/xselinux_hooks.c |   44 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/Xext/xselinux_hooks.c b/Xext/xselinux_hooks.c
index 0d4c9ab..c2b21d6 100644
--- a/Xext/xselinux_hooks.c
+++ b/Xext/xselinux_hooks.c
@@ -336,9 +336,17 @@ SELinuxDevice(CallbackListPtr *pcbl, pointer unused, pointer calldata)
     SELinuxAuditRec auditdata = { .client = rec->client, .dev = rec->dev };
     security_class_t cls;
     int rc;
+    DeviceIntPtr dev = NULL;
+    int i = 0;

     subj = dixLookupPrivate(&rec->client->devPrivates, subjectKey);
-    obj = dixLookupPrivate(&rec->dev->devPrivates, objectKey);
+    /*
+     * The XIAllMasterDevices or XIAllDevices do not have devPrivates
+     * entries. Therefore dixLookupPrivate for the object is done later
+     * for these device IDs.
+     */
+    if (rec->dev->id != XIAllDevices && rec->dev->id != XIAllMasterDevices)
+        obj = dixLookupPrivate(&rec->dev->devPrivates, objectKey);

     /* If this is a new object that needs labeling, do it now */
     if (rec->access_mode & DixCreateAccess) {
@@ -356,12 +364,38 @@ SELinuxDevice(CallbackListPtr *pcbl, pointer unused, pointer calldata)
    }
     }

-    cls = IsPointerDevice(rec->dev) ? SECCLASS_X_POINTER : SECCLASS_X_KEYBOARD;
-    rc = SELinuxDoCheck(subj, obj, cls, rec->access_mode, &auditdata);
-    if (rc != Success)
-    rec->status = rc;
+    if (rec->dev->id != XIAllDevices && rec->dev->id != XIAllMasterDevices) {
+           cls = IsPointerDevice(rec->dev) ? SECCLASS_X_POINTER : SECCLASS_X_KEYBOARD;
+           rc = SELinuxDoCheck(subj, obj, cls, rec->access_mode, &auditdata);
+           if (rc != Success)
+            rec->status = rc;
+        return;
+    } else {
+        /*
+         * Device ID must be 0 or 1
+         * We have to check both possible classes as we don't know whether it
+         * was a pointer or keyboard. Therefore all devices are checked for:
+         *         rec->dev->id == XIAllDevices
+         * and only masters for:
+         *         rec->dev->id == XIAllMasterDevices
+         *
+         * An error is returned should any device fail SELinuxDoCheck
+         */
+        for (dev = inputInfo.devices; dev; dev = dev->next, i++) {
+            if (!IsMaster(dev) && rec->dev->id == XIAllMasterDevices)
+                continue;
+              cls = IsPointerDevice(dev) ? SECCLASS_X_POINTER : SECCLASS_X_KEYBOARD;
+            obj = dixLookupPrivate(&dev->devPrivates, objectKey);
+            rc = SELinuxDoCheck(subj, obj, cls, rec->access_mode, &auditdata);
+            if (rc != Success) {
+                rec->status = rc;
+                return;
+            }
+        }
+    }
}

+
static void
SELinuxSend(CallbackListPtr *pcbl, pointer unused, pointer calldata)
{
-- 
1.7.10.2
Comment 4 Jeremy Huddleston Sequoia 2012-06-12 09:18:11 UTC
Richard sent that patch was to xorg-devel for review about 2 hours ago.

Thanks, Richard.
Comment 5 Peter Hutterer 2012-06-13 22:11:50 UTC
aforementioned patch on list: http://patchwork.freedesktop.org/patch/10621/
Comment 6 Adam Jackson 2018-06-13 17:38:34 UTC
Better version, which I'm pretty sure is still valid:

https://patchwork.freedesktop.org/patch/11097/
Comment 7 GitLab Migration User 2018-12-13 22:26:39 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/xserver/issues/423.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.