Investigating the Xorg crash originally reported at: https://bugzilla.gnome.org/show_bug.cgi?id=604343#c7 I found the problem was that in the Security extension at SecurityResource() we're trying to find the private in a NULL client: (dbx) where current thread: t@1 [1] privateExists(privates = 0xe8, key = 0x8128c0), line 81 in "privates.c" [2] dixLookupPrivate(privates = 0xe8, key = 0x8128c0), line 162 in "privates.c" =>[3] SecurityResource(pcbl = 0x7489c0, unused = (nil), calldata = 0xfffffd7fffdffa98), line 818 in "security.c" [4] _CallCallbacks(pcbl = 0x7489c0, call_data = 0xfffffd7fffdffa98), line 737 in "dixutils.c" [5] CallCallbacks(pcbl = 0x7489c0, call_data = 0xfffffd7fffdffa98), line 871 in "dixutils.c" [6] XaceHook(hook = 2, ... = 0x55aa65, ...), line 212 in "xace.c" [7] ProcXFixesGetCursorImageAndName(client = 0x1381f90), line 544 in "cursor.c" [8] ProcXFixesDispatch(client = 0x1381f90), line 163 in "xfixes.c" [9] Dispatch(), line 449 in "dispatch.c" [10] main(argc = 9, argv = 0xfffffd7fffdffcc8, envp = 0xfffffd7fffdffd18), line 332 in "main.c" (dbx) frame 3 Current function is SecurityResource 818 obj = dixLookupPrivate(&clients[cid]->devPrivates, stateKey); (dbx) print cid cid = 8 (dbx) print clients[cid] clients[cid] = (nil) We appear to be trying to check the permission on a cursor set by the client which already exited. After re-running to capture the client ids when debugging, it appears it's the metacity client that gdm was running in it's greeter session that was there and had set the cursor before exiting.
Created attachment 32466 [details] [review] patch to Xext/security.c from Xorg 1.7.3 This patch stops the crashes by failing any attempt to access the resources of an exited client - which is probably not the right solution, and doesn't solve the problem of checking the wrong client's private when a new client connects and gets the same id. I'm not sure what needs to happen to resources like cursors left behind by exited clients to get the right security state.
I think the solution to this is to copy the trustLevel information into the object's own devPrivates. This is only possible when the resource object has a devPrivates field, so it is not a complete solution. However the Cursor object does have one, and I have not seen any similar crashes with the SELinux extension which uses the same method. Here is a patch that does this. An alternative solution would be to not use devPrivates at all. Instead, a static array would hold the trustLevel for each client. About the issue of a new client connecting and getting the same base ID: isn't this a problem to begin with, because the new client would end up owning the old resource?
Created attachment 32476 [details] [review] alternate patch to Xext/security.c Patch that stores the trustLevel information in the resource object itself when possible. This will allow security checks to be made after the client structure has gone away (at least on resource objects that have a devPrivates field).
What is the status of this bug now? Is the patch committed?
> What is the status of this bug now? Is the patch committed? A version of the simple crash prevention patch was committed for Xorg 1.9: http://cgit.freedesktop.org/xorg/xserver/commit/?id=1c08a37e0eb4746e8974eb7a70ca4b7b84712963 Leaving the bug open to consider Eamon's better fix.
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.