[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen/grants: fix hypercall continuation for GNTTABOP_cache_flush



On Wed, 22 Apr 2020, Jan Beulich wrote:
> On 22.04.2020 15:07, Juergen Gross wrote:
> > --- a/xen/common/grant_table.c
> > +++ b/xen/common/grant_table.c
> > @@ -3626,12 +3626,12 @@ do_grant_table_op(
> >          if ( unlikely(!guest_handle_okay(cflush, count)) )
> >              goto out;
> >          rc = gnttab_cache_flush(cflush, &opaque_in, count);
> > -        if ( rc > 0 )
> > +        if ( rc >= 0 )
> >          {
> >              guest_handle_add_offset(cflush, rc);
> >              uop = guest_handle_cast(cflush, void);
> > +            opaque_out = opaque_in;
> >          }
> > -        opaque_out = opaque_in;
> >          break;
> >      }
> >  
> > @@ -3641,7 +3641,7 @@ do_grant_table_op(
> >      }
> >  
> >    out:
> > -    if ( rc > 0 || opaque_out != 0 )
> > +    if ( rc > 0 || (opaque_out != 0 && rc == 0) )
> 
> I disagree with this part - opaque_out shouldn't end up non-zero
> when rc < 0, and it won't anymore with the change in the earlier
> hunk.

But opaque_out could end up being non-zero when rc == 0. I think it is a
good improvement that we explicitly prevent rc < 0 from entering this
if condition. This is why this new version of the patch is my favorite:
it is the one that leads to the code most robust. 

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


That said, as I mentioned before, I would be OK even without the last
part because it would still be enough to fix the bug.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.