[PATCH xserver] present: Be consistent in using window vs vblank->window in present_execute()

[Chris Wilson]

Upon entering the function, we copy frequently accessed members of the
vblank structure to locals (such as the Window). However, we then
fluctuate between using the local window and the vblank->window. Under
certain situations, the present_flip callback into the driver may be
completed instantaneously and so accessing the vblank structure after a
successful call into the driver may cause a use-after-free. This is
trivially avoided by using the locals we took earlier.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 present/present.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/present/present.c b/present/present.c
index 105e2bf..a90f08d 100644
--- a/present/present.c
+++ b/present/present.c
@@ -628,6 +628,7 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
 {
     WindowPtr                   window = vblank->window;
     ScreenPtr                   screen = window->drawable.pScreen;
+    PixmapPtr                   pixmap = vblank->pixmap;
     present_screen_priv_ptr     screen_priv = present_screen_priv(screen);
     uint8_t                     mode;
 
@@ -648,7 +649,7 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
         }
     }
 
-    if (vblank->flip && vblank->pixmap && vblank->window) {
+    if (vblank->flip && pixmap && window) {
         if (screen_priv->flip_pending || screen_priv->unflip_event_id) {
             DebugPresent(("\tr %lld %p (pending %p unflip %lld)\n",
                           vblank->event_id, vblank,
@@ -662,13 +663,14 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
     xorg_list_del(&vblank->window_list);
     vblank->queued = FALSE;
 
-    if (vblank->pixmap && vblank->window) {
+    if (pixmap && window) {
 
         if (vblank->flip) {
+	    RegionPtr damage = vblank->update;
 
             DebugPresent(("\tf %lld %p %8lld: %08lx -> %08lx\n",
                           vblank->event_id, vblank, crtc_msc,
-                          vblank->pixmap->drawable.id, vblank->window->drawable.id));
+                          id, window->drawable.id));
 
             /* Prepare to flip by placing it in the flip queue and
              * and sticking it into the flip_pending field
@@ -678,8 +680,7 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
             xorg_list_add(&vblank->event_queue, &present_flip_queue);
             /* Try to flip
              */
-            if (present_flip(vblank->crtc, vblank->event_id, vblank->target_msc, vblank->pixmap, vblank->sync_flip)) {
-                RegionPtr damage;
+            if (present_flip(vblank->crtc, vblank->event_id, vblank->target_msc, pixmap, vblank->sync_flip)) {
 
                 /* Fix window pixmaps:
                  *  1) Restore previous flip window pixmap
@@ -689,18 +690,17 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
                     present_set_tree_pixmap(screen_priv->flip_window,
                                             screen_priv->flip_pixmap,
                                             (*screen->GetScreenPixmap)(screen));
-                present_set_tree_pixmap(vblank->window, NULL, vblank->pixmap);
-                present_set_tree_pixmap(screen->root, NULL, vblank->pixmap);
+                present_set_tree_pixmap(window, NULL, pixmap);
+                present_set_tree_pixmap(screen->root, NULL, pixmap);
 
                 /* Report update region as damaged
                  */
-                if (vblank->update) {
-                    damage = vblank->update;
+                if (damage)
                     RegionIntersect(damage, damage, &window->clipList);
-                } else
+                else
                     damage = &window->clipList;
 
-                DamageDamageRegion(&vblank->window->drawable, damage);
+                DamageDamageRegion(&window->drawable, damage);
                 return;
             }
 
@@ -710,7 +710,7 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
             screen_priv->flip_pending = NULL;
             vblank->flip = FALSE;
         }
-        DebugPresent(("\tc %p %8lld: %08lx -> %08lx\n", vblank, crtc_msc, vblank->pixmap->drawable.id, vblank->window->drawable.id));
+        DebugPresent(("\tc %p %8lld: %08lx -> %08lx\n", vblank, crtc_msc, pixmap->drawable.id, window->drawable.id));
         if (screen_priv->flip_pending) {
 
             /* Check pending flip
@@ -738,7 +738,7 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
             return;
         }
 
-        present_copy_region(&window->drawable, vblank->pixmap, vblank->update, vblank->x_off, vblank->y_off);
+        present_copy_region(&window->drawable, pixmap, vblank->update, vblank->x_off, vblank->y_off);
 
         /* present_copy_region sticks the region into a scratch GC,
          * which is then freed, freeing the region
@@ -746,13 +746,13 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
         vblank->update = NULL;
         present_flush(window);
 
-        present_pixmap_idle(vblank->pixmap, vblank->window, vblank->serial, vblank->idle_fence);
+        present_pixmap_idle(pixmap, window, vblank->serial, vblank->idle_fence);
     }
 
     /* Compute correct CompleteMode
      */
     if (vblank->kind == PresentCompleteKindPixmap) {
-        if (vblank->pixmap && vblank->window)
+        if (pixmap && window)
             mode = PresentCompleteModeCopy;
         else
             mode = PresentCompleteModeSkip;
-- 
2.8.0.rc3