[Libguestfs] [nbdkit PATCH 2/5] retry: Check size before transactions

[Richard W.M. Jones]

On Fri, Oct 04, 2019 at 01:31:49PM +0100, Richard W.M. Jones wrote:
> On Thu, Oct 03, 2019 at 09:54:37PM -0500, Eric Blake wrote:
> > Although it is unusual for a plugin to shrink size on reopen, it is
> > not impossible.  Failure to check our bounds could result in violating
> > assumptions in the plugin that all requests are in-bounds.  Note that
> > if the plugin gains a larger size, we merely never access the new tail
> > of the file (when the NBD protocol ever gains dynamic resize support,
> > we'll get to revisit how the retry filter fits in).
> > 
> > Fixes: f0f0ec49
> > Signed-off-by: Eric Blake <eblake at redhat.com>
> 
> I'm inclined to say we should document that the retry filter only
> works with reasonable plugins and document those assumptions, but as
> you've written the code now let's go with this.

For example one thing we might {do,have done} is to check that things
like size, can_* etc don't change after reopening.

However you've written the code and added the tests and it all looks
good, so:

ACK series

Thanks,

Rich.

> Rich.
> 
> >  filters/retry/retry.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/filters/retry/retry.c b/filters/retry/retry.c
> > index 840d7383..cf8f5246 100644
> > --- a/filters/retry/retry.c
> > +++ b/filters/retry/retry.c
> > @@ -148,6 +148,17 @@ struct retry_data {
> >    int delay;                    /* Seconds to wait before retrying. */
> >  };
> > 
> > +static bool
> > +valid_range (struct nbdkit_next_ops *next_ops, void *nxdata,
> > +             uint32_t count, uint64_t offset, bool is_write, int *err)
> > +{
> > +  if ((int64_t) offset + count > next_ops->get_size (nxdata)) {
> > +    *err = is_write ? ENOSPC : EIO;
> > +    return false;
> > +  }
> > +  return true;
> > +}
> > +
> >  static bool
> >  do_retry (struct retry_handle *h,
> >            struct retry_data *data,
> > @@ -204,6 +215,8 @@ retry_pread (struct nbdkit_next_ops *next_ops, void *nxdata,
> >    int r;
> > 
> >   again:
> > +  if (! valid_range (next_ops, nxdata, count, offset, false, err))
> > +    return -1;
> >    r = next_ops->pread (nxdata, buf, count, offset, flags, err);
> >    if (r == -1 && do_retry (h, &data, next_ops, nxdata, err)) goto again;
> > 
> > @@ -226,6 +239,8 @@ retry_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata,
> >      *err = EROFS;
> >      return -1;
> >    }
> > +  if (! valid_range (next_ops, nxdata, count, offset, true, err))
> > +    return -1;
> >    if (next_ops->can_write (nxdata) != 1) {
> >      *err = EROFS;
> >      r = -1;
> > @@ -258,6 +273,8 @@ retry_trim (struct nbdkit_next_ops *next_ops, void *nxdata,
> >      *err = EROFS;
> >      return -1;
> >    }
> > +  if (! valid_range (next_ops, nxdata, count, offset, true, err))
> > +    return -1;
> >    if (next_ops->can_trim (nxdata) != 1) {
> >      *err = EROFS;
> >      r = -1;
> > @@ -312,6 +329,8 @@ retry_zero (struct nbdkit_next_ops *next_ops, void *nxdata,
> >      *err = EROFS;
> >      return -1;
> >    }
> > +  if (! valid_range (next_ops, nxdata, count, offset, true, err))
> > +    return -1;
> >    if (flags & NBDKIT_FLAG_FAST_ZERO &&
> >             next_ops->can_fast_zero (nxdata) != 1) {
> >      *err = EOPNOTSUPP;
> > @@ -347,6 +366,8 @@ retry_extents (struct nbdkit_next_ops *next_ops, void *nxdata,
> >    size_t i;
> > 
> >   again:
> > +  if (! valid_range (next_ops, nxdata, count, offset, false, err))
> > +    return -1;
> >    if (next_ops->can_extents (nxdata) != 1) {
> >      *err = EIO;
> >      r = -1;
> > @@ -390,6 +411,8 @@ retry_cache (struct nbdkit_next_ops *next_ops, void *nxdata,
> >    int r;
> > 
> >   again:
> > +  if (! valid_range (next_ops, nxdata, count, offset, false, err))
> > +    return -1;
> >    if (next_ops->can_cache (nxdata) <= NBDKIT_CACHE_NONE) {
> >      *err = EIO;
> >      r = -1;
> > -- 
> > 2.21.0
> > 
> > _______________________________________________
> > Libguestfs mailing list
> > Libguestfs at redhat.com
> > https://www.redhat.com/mailman/listinfo/libguestfs
> 
> -- 
> Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> virt-builder quickly builds VMs from scratch
> http://libguestfs.org/virt-builder.1.html

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v