Re: [PATCH 4/7] drm/xe/oa: Signal output fences

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 17 Sep 2024 16:38:43 -0700, Matthew Brost wrote:
>
> > > +	dma_fence_init(&ofence->base, &xe_oa_fence_ops, &stream->oa_fence_lock, 0, 0);
> >
> > About stream->oa_fence_lock. This was added in response to this comment
> > from you in v1 of this patch:
> >
> >	/* No per fence spin lock, global OA one for user_signal */
> >
> > here:
> >	https://patchwork.freedesktop.org/patch/607602/?series=137058&rev=1#comment_1104327
> >
> > However, I just realized that, the way it is implemented, there is a uaf
> > here: lock memory can be freed before the lock is accessed in
> > dma_fence_signal() in xe_oa_fence_work_fn() (when the oa stream is
> > destroyed).
> >
> > Of course we can implement some reference counting etc. to solve the uaf,
> > but the simplest way to solve it seems to be to go back to the per-fence
> > lock, unless doing so is incorrect? Looking at dma_fence_init calls, I do
> > see per fence locks being used occasionally (xe_preempt_fence, vgem_fence
> > etc.).
> >
> > So just want to check with you if it's ok to go back to the per fence lock,
> > or we should solve it some other way.
> >
>
> Yea my bad on that suggestion. That is an issue, just move the lock back
> to the fence.

No problem, thanks, I've moved the lock back into the fence in the latest
version.

Ashutosh