[PATCH v2] present: fix freed pointer access

[Olivier Fourdan]

Hi,

On Wed, Sep 5, 2018 at 1:13 PM Olivier Fourdan <ofourdan at redhat.com> wrote:
> Hi,
> On Tue, Sep 4, 2018 at 6:28 PM Lionel Landwerlin
> <lionel.g.landwerlin at intel.com> wrote:
> >
> > Oh well...
> > I'm sure you'll be able to fix it faster than me :)
> >
> > -
> > Lionel
> >
> >
> [...]
>
> If I read bug 107314 correctly, the crash occurs after the window has
> been destroyed, so what about that other patch:
>
> https://patchwork.freedesktop.org/patch/247271/
>
> ** plus ** this patch below (just copied for testing purpose), does it
> fix your crash?

Sorry, that's unrelated, please ignore my last post...

So I agree with Lionel's v2 patch
(https://patchwork.freedesktop.org/patch/246007/) to fix the issue
with the use-after-free.

> > On 04/09/2018 16:27, Roman Gilg wrote:
> >
> > Ok, I just got a failing assert in xwl_present_flips_stop with the patch when opening a context menu in Steam.
> > Seems the xwl_present_flips_stop call is coming in too late now after the presenting window has already been
> > changed.

I fail to understand how that can be related to Lionel's patch though.

The patch simply moves `present_vblank_notify()` before
`present_wnmd_flips_stop()` in `present_wnmd_flip_notify()` whereas
the assertion failure comes from an eniterly different code path, down
from `present_clip_notify()` which calls
`present_wnmd_check_flip_window()` which does:

```
325     if (flip_pending) {
326         if (!present_wnmd_check_flip(flip_pending->crtc,
flip_pending->window, flip_pending->pixmap,
327                                 flip_pending->sync_flip, NULL, 0, 0, NULL))
328             present_wnmd_set_abort_flip(window);
329     } else if (flip_active) {
330         if (!present_wnmd_check_flip(flip_active->crtc,
flip_active->window, flip_active->pixmap,
331                                      flip_active->sync_flip, NULL,
0, 0, NULL))
332             present_wnmd_flips_stop(window);
333     }
```

`present_wnmd_check_flip()` calls `xwl_present_check_flip2()` which does:

```
424      * Do not flip if there is already another child window doing flips.
425      */
426     if (xwl_window->present_window &&
427             xwl_window->present_window != present_window)
428         return FALSE;
429
```

So here we end up with `assert(xwl_window->present_window == window)`
in `xwl_present_flips_stop()` when `present_wnmd_check_flip()` returns
FALSE, i.e. when `xwl_window->present_window != present_window`.

I'm confused :/

Cheers,
Olivier