summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Hutterer <peter.hutterer@who-t.net>2022-11-30 11:20:40 +1000
committerPeter Hutterer <peter.hutterer@who-t.net>2022-12-14 11:24:41 +1000
commit67927cc41f452228188bbe2aa34a9ee4a9ce0c6b (patch)
tree5c1750032e3eb38eade07afdf72c8137f844c9a2
parenta6c0d7b142e762a6b9934a23e060ea91ff5afcea (diff)
downloadxserver-67927cc41f452228188bbe2aa34a9ee4a9ce0c6b.tar.gz
Xext: free the XvRTVideoNotify when turning off from the same client
This fixes a use-after-free bug: When a client first calls XvdiSelectVideoNotify() on a drawable with a TRUE onoff argument, a struct XvVideoNotifyRec is allocated. This struct is added twice to the resources: - as the drawable's XvRTVideoNotifyList. This happens only once per drawable, subsequent calls append to this list. - as the client's XvRTVideoNotify. This happens for every client. The struct keeps the ClientPtr around once it has been added for a client. The idea, presumably, is that if the client disconnects we can remove all structs from the drawable's list that match the client (by resetting the ClientPtr to NULL), but if the drawable is destroyed we can remove and free the whole list. However, if the same client then calls XvdiSelectVideoNotify() on the same drawable with a FALSE onoff argument, only the ClientPtr on the existing struct was set to NULL. The struct itself remained in the client's resources. If the drawable is now destroyed, the resource system invokes XvdiDestroyVideoNotifyList which frees the whole list for this drawable - including our struct. This function however does not free the resource for the client since our ClientPtr is NULL. Later, when the client is destroyed and the resource system invokes XvdiDestroyVideoNotify, we unconditionally set the ClientPtr to NULL. On a struct that has been freed previously. This is generally frowned upon. Fix this by calling FreeResource() on the second call instead of merely setting the ClientPtr to NULL. This removes the struct from the client resources (but not from the list), ensuring that it won't be accessed again when the client quits. Note that the assignment tpn->client = NULL; is superfluous since the XvdiDestroyVideoNotify function will do this anyway. But it's left for clarity and to match a similar invocation in XvdiSelectPortNotify. CVE-2022-46342, ZDI-CAN 19400 This vulnerability was discovered by: Jan-Niklas Sohn working with Trend Micro Zero Day Initiative Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> Acked-by: Olivier Fourdan <ofourdan@redhat.com> (cherry picked from commit b79f32b57cc0c1186b2899bce7cf89f7b325161b)
-rw-r--r--Xext/xvmain.c4
1 files changed, 3 insertions, 1 deletions
diff --git a/Xext/xvmain.c b/Xext/xvmain.c
index f62747193..2a08f8744 100644
--- a/Xext/xvmain.c
+++ b/Xext/xvmain.c
@@ -811,8 +811,10 @@ XvdiSelectVideoNotify(ClientPtr client, DrawablePtr pDraw, BOOL onoff)
tpn = pn;
while (tpn) {
if (tpn->client == client) {
- if (!onoff)
+ if (!onoff) {
tpn->client = NULL;
+ FreeResource(tpn->id, XvRTVideoNotify);
+ }
return Success;
}
if (!tpn->client)