frederik
/
xserver
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

(submit/fix-SelectSelectionInput) dix: create empty selection objects as-needed in dixLookupSelection() 

For now, new selection objects are only created in ProcSetSelectionOwner()
when dixLookupSelection() can't find the requested one (returns BadMatch).

When somebody's trying to listen on a not-yet existing selection, via
XFixesSelectSelectionInput() -- XFIXES:SelectSelectionInput message -- he's
also getting BadMatch. This isn't neccessarily completely wrong, the spec
doesn't really tell anything about those situations (it doens't tell anything
about selection's lifetimes, just their ownerships). But there are real-
world clients not expecting an error here and crashing - the problem popped
up just recently, due to a necessary security fix (remote memory corruption
plus XACE missing to catch SelectSelectionInput) that alread went unnoticed
for far too long (*1).

So, it's better being polite and interpret the spec in the way that any
potential selection exists as soon as it's used by someone. (in fact, they
never get deleted anyways, just cleared).

XACE consumers get properly notified by the new Selection object creation
(eg. SElinux is attaching it's private data to it). And all callers already
prepared to get a cleared Selection object, because that's always been a
perfectly normal situation - Selection objects never get removed again,
just cleared.

*1) 601fd0fd84

X-Fixes: 601fd0fd84
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This commit is contained in:
Enrico Weigelt, metux IT consult 2024-06-26 17:21:01 +02:00
parent 8d525b6ade
commit 52f11ad5c2
1 changed files with 31 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
dix/selection.c
View File

@ -82,8 +82,20 @@ dixLookupSelection(Selection ** result, Atom selectionName,
if (pSel->selection == selectionName)
break;
if (pSel)
rc = XaceHookSelectionAccess(client, &pSel, access_mode);
if (!pSel) {
if (!(pSel = dixAllocateObjectWithPrivates(Selection, PRIVATE_SELECTION)))
return BadAlloc;
pSel->selection = selectionName;
pSel->next = CurrentSelections;
CurrentSelections = pSel;
}
/* security creation/labeling check */
rc = XaceHookSelectionAccess(client, &pSel, access_mode | DixCreateAccess);
if (rc != Success) {
return rc;
}
*result = pSel;
return rc;
}
@ -175,7 +187,9 @@ ProcSetSelectionOwner(ClientPtr client)
*/
rc = dixLookupSelection(&pSel, stuff->selection, client, DixSetAttrAccess);
if (rc == Success) {
if (rc != Success)
return rc;
/* If the timestamp in client's request is in the past relative
to the time stamp indicating the last time the owner of the
selection was set, do not set the selection, just return
@ -191,30 +205,6 @@ ProcSetSelectionOwner(ClientPtr client)
event.u.u.type = SelectionClear;
WriteEventsToClient(pSel->client, 1, &event);
}
}
else if (rc == BadMatch) {
/*
* It doesn't exist, so add it...
*/
pSel = dixAllocateObjectWithPrivates(Selection, PRIVATE_SELECTION);
if (!pSel)
return BadAlloc;
pSel->selection = stuff->selection;
/* security creation/labeling check */
rc = XaceHookSelectionAccess(client, &pSel,
DixCreateAccess | DixSetAttrAccess);
if (rc != Success) {
free(pSel);
return rc;
}
pSel->next = CurrentSelections;
CurrentSelections = pSel;
}
else
return rc;
pSel->lastTimeChanged = time;
pSel->window = stuff->window;