[PATCH] dma-buf: fix stack corruption in dma_fence_chain_release
Daniel Vetter
daniel at ffwll.ch
Mon Aug 5 16:30:38 UTC 2019
On Mon, Aug 05, 2019 at 04:32:20PM +0300, Lionel Landwerlin wrote:
> That one test creates a 32k chain of fences I think.
> Anyway my kernel crash was unrelated ;)
Hm I'd expect that with clever use of vgem fake fences we should be able
to repro this bug here with an igt. Would be real nice, any takers?
-Daniel
>
> -Lionel
>
> On 05/08/2019 16:02, Christian König wrote:
> > Not even remotely :)I tested this with my own crafted code inside the
> > kernel.
> >
> > It's probably quite some hassle to actually trigger this problem from
> > userspace and I only found it because I created a very very long
> > sequence chain by accident.
> >
> > Christian.
> >
> > Am 05.08.19 um 14:03 schrieb Lionel Landwerlin:
> > > By any change, did you run into this with a CTS test whose name ends
> > > with ".chain" ? :)
> > >
> > > -Lionel
> > >
> > > On 05/08/2019 10:36, Christian König wrote:
> > > > We can't free up the chain using recursion or we run into a
> > > > stack overflow.
> > > >
> > > > Manually free up the dangling chain nodes to avoid recursion.
> > > >
> > > > Signed-off-by: Christian König <christian.koenig at amd.com>
> > > > ---
> > > > drivers/dma-buf/dma-fence-chain.c | 24 +++++++++++++++++++++++-
> > > > 1 file changed, 23 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/dma-buf/dma-fence-chain.c
> > > > b/drivers/dma-buf/dma-fence-chain.c
> > > > index b5089f64be2a..44a741677d25 100644
> > > > --- a/drivers/dma-buf/dma-fence-chain.c
> > > > +++ b/drivers/dma-buf/dma-fence-chain.c
> > > > @@ -178,8 +178,30 @@ static bool dma_fence_chain_signaled(struct
> > > > dma_fence *fence)
> > > > static void dma_fence_chain_release(struct dma_fence *fence)
> > > > {
> > > > struct dma_fence_chain *chain = to_dma_fence_chain(fence);
> > > > + struct dma_fence *prev;
> > > > +
> > > > + /* Manually unlink the chain as much as possible to avoid
> > > > recursion
> > > > + * and potential stack overflow.
> > > > + */
> > > > + while ((prev = rcu_dereference_protected(chain->prev, true))) {
> > > > + struct dma_fence_chain *prev_chain;
> > > > +
> > > > + if (kref_read(&prev->refcount) > 1)
> > > > + break;
> > > > +
> > > > + prev_chain = to_dma_fence_chain(prev);
> > > > + if (!prev_chain)
> > > > + break;
> > > > +
> > > > + /* No need for atomic operations since we hold the last
> > > > + * reference to prev_chain.
> > > > + */
> > > > + chain->prev = prev_chain->prev;
> > > > + RCU_INIT_POINTER(prev_chain->prev, NULL);
> > > > + dma_fence_put(prev);
> > > > + }
> > > > + dma_fence_put(prev);
> > > > - dma_fence_put(rcu_dereference_protected(chain->prev, true));
> > > > dma_fence_put(chain->fence);
> > > > dma_fence_free(fence);
> > > > }
> > >
> > >
> >
> >
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list