Hi, Looks good to me, although I thought this was handled at the callsites. I guess some callsites have been added or changed that pass in the special ID's.
Acked-by: Eamon Walsh <[email protected]> On Tue, Jun 12, 2012 at 9:49 AM, Richard Haines < [email protected]> wrote: > This patch was created using xorg-server-1.12.0 source. > > When using Fedora 17 with xorg-server-1.12.0 and SELinux is enabled > ('setsebool xserver_object_manager on') the xserver will not load. The X > log file has a seg fault pointing to XACE/SELinux. Bug 50641 was raised > (https://bugs.freedesktop.org/show_bug.cgi?id=50641). The patch below is a > possible fix. > > 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 <[email protected]> > --- > 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 > > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel >
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
