On Wed, 2026-09-02 at 08:56 +0200, Jiri Slaby wrote:
> On 02. 09. 26, 8:52, Philipp Stanner wrote:
> > On Fri, 2026-08-28 at 10:32 +0200, Jiri Slaby wrote:
> > > On 28. 08. 26, 9:46, Jiri Slaby wrote:
> > > > What helps is indeed the revert back to:
> > > > 
> > > > --- a/drivers/gpu/drm/qxl/qxl_release.c
> > > > +++ b/drivers/gpu/drm/qxl/qxl_release.c
> > > > @@ -147,7 +147,7 @@ qxl_release_free(struct qxl_device *qdev,
> > > >           idr_remove(&qdev->release_idr, release->id);
> > > >           spin_unlock(&qdev->release_idr_lock);
> > > > 
> > > > -       if (dma_fence_was_initialized(&release->base)) {
> > > > +       if (release->base.ops) {
> > > 
> > > Crap, the flags simple got garbage from previous freed buffer, so:
> > > -       release = kmalloc(size, GFP_KERNEL);
> > > +       release = kzalloc(size, GFP_KERNEL);
> > > 
> > > fixes this of course. Patch coming.
> > > 
> > > Now, the ops and initialized are either both set or unset:
> > > qxl_release_free 1: release->base.ops=0x0 initialized=0 id=6 type=2
> > > qxl_release_free 1: release->base.ops=qxl_fence_ops initialized=1 id=2
> > > type=0
> > > 
> > > Unlike before.
> > > 
> > > FTR release->base.ops were NULLed after that kmalloc. Unlike
> > > release->base.flags.
> > 
> > You're saying that QXL is re-using that fence if it were uninitialized?
> 
> Sort of. The fence was dropped and freed. Then a new memory was 
> allocated. And garbage from that allocation was used -- it might be the 
> old (freed) fence. Or anything else freed previously. Simply garbage.


Maybe even dma_fence_init() should zero out the memory for maximum
robustness.

We do that in drm_sched at a few places.

Opinions? @Christian?


P.

Reply via email to