[RFC][PATCH] dix/resource: fix use after free in resource code with DRI

[Adam Jackson]

On Wed, 2009-06-10 at 13:34 +1000, Dave Airlie wrote:
> LookupClientResourceComplex is used by DRI1 code to find and free a DRI
> drawable in a callback, however when the DRI code returns this->value
> is now pointing at freed memory. It seemed easiest to store the value
> to a temporary and return it afterwards.
> 
> Another option might be a new FreeClientResourceComplex or one that
> also returns the id, so we can free it using an alternative means.
> 
> found using valgrind, any comments?

Better, but.  If the callback allocates as well as frees, and then
returns FALSE, the region pointed to by 'this' could get reused, in
which case this->next would be garbage.

Should be more like this I think?

--- a/dix/resource.c
+++ b/dix/resource.c
@@ -707,7 +707,8 @@ LookupClientResourceComplex(
     pointer cdata
 ){
     ResourcePtr *resources;
-    ResourcePtr this;
+    ResourcePtr this, next;
+    pointer value;
     int i;
 
     if (!client)
@@ -715,10 +716,12 @@ LookupClientResourceComplex(
 
     resources = clientTable[client->index].resources;
     for (i = 0; i < clientTable[client->index].buckets; i++) {
-        for (this = resources[i]; this; this = this->next) {
+        for (this = resources[i]; this; this = next) {
+           next = this->next;
            if (!type || this->type == type) {
+               value = this->value;
                if((*func)(this->value, this->id, cdata))
-                   return this->value;
+                   return value;
            }
        }
     }

- ajax
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : http://lists.x.org/archives/xorg-devel/attachments/20090610/e2c282ae/attachment.pgp