Re: [Xen-devel] [PATCH] xen/pvcalls: fix potential endless loop in pvcalls-front.c

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 6 Nov 2017, Boris Ostrovsky wrote:
> On 11/06/2017 05:17 PM, Stefano Stabellini wrote:
> > mutex_trylock() returns 1 if you take the lock and 0 if not. Assume you
> > take in_mutex on the first try, but you can't take out_mutex. Next times
> > you call mutex_trylock() in_mutex is going to fail. It's an endless
> > loop.
> >
> > Solve the problem by moving the two mutex_trylock calls to two separate
> > loops.
> >
> > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > CC: boris.ostrovsky@xxxxxxxxxx
> > CC: jgross@xxxxxxxx
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> 
> BTW, I assume that when recvmsg or sendmsg is called no other locks are
> held, right? (otherwise we'd get into a deadlock.)

Yes, but most importantly the other assumption is that recvmsg and
sendmsg are already running: the partially visible comment explains it:

    * Wait until there are no more waiters on the mutexes.
        * We know that no new waiters can be added because sk_send_head
        * is set to NULL -- we only need to wait for the existing
        * waiters to return.


> > ---
> >  drivers/xen/pvcalls-front.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
> > index 0c1ec68..047dce7 100644
> > --- a/drivers/xen/pvcalls-front.c
> > +++ b/drivers/xen/pvcalls-front.c
> > @@ -1048,8 +1048,9 @@ int pvcalls_front_release(struct socket *sock)
> >              * is set to NULL -- we only need to wait for the existing
> >              * waiters to return.
> >              */
> > -           while (!mutex_trylock(&map->active.in_mutex) ||
> > -                      !mutex_trylock(&map->active.out_mutex))
> > +           while (!mutex_trylock(&map->active.in_mutex))
> > +                   cpu_relax();
> > +           while (!mutex_trylock(&map->active.out_mutex))
> >                     cpu_relax();
> >  
> >             pvcalls_front_free_map(bedata, map);
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel