[PATCH] dma-buf/sw_sync: Fix timeline/pt overflow cases

[Chris Wilson]

Quoting Sean Paul (2017-06-28 17:47:24)
> On Wed, Jun 28, 2017 at 05:00:20PM +0100, Chris Wilson wrote:
> > Quoting Sean Paul (2017-06-28 16:51:11)
> > > Protect against long-running processes from overflowing the timeline
> > > and creating fences that go back in time. While we're at it, avoid
> > > overflowing while we're incrementing the timeline.
> > > 
> > > Signed-off-by: Sean Paul <seanpaul at chromium.org>
> > > ---
> > >  drivers/dma-buf/sw_sync.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> > > index 69c5ff36e2f9..40934619ed88 100644
> > > --- a/drivers/dma-buf/sw_sync.c
> > > +++ b/drivers/dma-buf/sw_sync.c
> > > @@ -142,7 +142,7 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
> > >  
> > >         spin_lock_irqsave(&obj->child_list_lock, flags);
> > >  
> > > -       obj->value += inc;
> > > +       obj->value += min(inc, ~0x0U - obj->value);
> > 
> > The timeline uses u32 seqno, so just obj->value += min(inc, INT_MAX);
> > 
> Hi Chris,
> Thanks for the review.
> 
> I don't think that solves the same problem I was trying to solve. The issue is
> that android userspace increments value by 0x7fffffff twice in order to ensure
> all fences have signaled. This is causing value to overflow and is_signaled will
> never be true. With your snippet, the possibility of overflow still exists.
> 
> > Better of course would be to report the error,
> 
> AFAIK, it's not an error to jump the timeline, perhaps just bad taste. Capping
> value at UINT_MAX will ensure all fences are signaled, and the check below ensures
> that fences can't be created beyond that (returning an error at that point in
> time).

UINT_MAX doesn't imply all fences will be signaled either, the timeline
is supposed to wrap.

The issue is timeline_fence_signaled() is using the wrong test, it
should be return (int)(fence->seqno - parent->value) <= 0; If it helps
extract a little helper from dma_fence_is_later().
-Chris