KMS Legacy Cursor Support

[Maxime Ripard]

Hi Daniel,

I'm using this thread as the occasion to discuss the legacy cursor stuff
a bit further.

I've been trying to address the issue detailed here:
https://lore.kernel.org/all/20220221134155.125447-1-maxime@cerno.tech/

And the last patch in particular:
https://lore.kernel.org/all/20220221134155.125447-9-maxime@cerno.tech/

On Wed, Mar 30, 2022 at 11:45:17AM +0200, Daniel Vetter wrote:
> On Tue, Mar 15, 2022 at 12:53:30AM +0300, Dmitry Osipenko wrote:
> > On 3/11/22 17:22, Maxime Ripard wrote:
> > > Hi Dmitry,
> > > 
> > > On Thu, Mar 10, 2022 at 03:33:07AM +0300, Dmitry Osipenko wrote:
> > >> I was playing/testing SuperTuxKart using VirtIO-GPU driver and spotted a
> > >> UAF bug in drm_atomic_helper_wait_for_vblanks().
> > >>
> > >> SuperTuxKart can use DRM directly, i.e. you can run game in VT without
> > >> Xorg or Wayland, this is where bugs happens. SuperTuxKart uses a
> > >> non-blocking atomic page flips and UAF happens when a new atomic state
> > >> is committed while there is a previous page flip still in-fly.
> > >>
> > >> What happens is that the new and old atomic states refer to the same
> > >> CRTC state somehow. Once the older atomic state is destroyed, the CRTC
> > >> state is freed and the newer atomic state continues to use the freed
> > >> CRTC state.
> > > 
> > > I'm not sure what you mean by "the new and old atomic states refer to
> > > the same CRTC state", are those the same pointers?
> > 
> > Yes, the pointers are the same. I'd assume that the newer atomic state
> > should duplicate CRTC state, but apparently it doesn't happen.
> 
> The legacy cursor hack stuff does this, and it pretty fundamentally breaks
> everything. Might be good to retest with that disabled:
> 
> https://lore.kernel.org/dri-devel/20201023123925.2374863-1-daniel.vetter@ffwll.ch/
> 
> The problem is a bit that this might cause some regressions, for drivers
> which don't yet have the fancy new cursor fastpath for plane updates.

I've been trying to force it to be disabled in either atomic_check or
atomic_setup, and it was either slow (check), or ineffective (setup).

It's not really clear to me what this hack is about, and what we could
do to make sure the cursor plane gets updated without waiting for
vblank. vc4 has async plane update support, so I'm sure it's just some
plumbing to do but I'm not sure where and why

Thanks!
Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220331/3c7ecac1/attachment-0001.sig>