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

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(a)redhat.com&gt; 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.

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(a)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