On Wed, Aug 28, 2019 at 04:09:24PM -0500, Eric Blake wrote:
The man page claims both offset and range are optional (matching the code), but the --help text claims offset is mandatory, and the comment to the no-op offset_config_complete claims we require both parameters. We did not check for an offset larger than the underlying size when there was no range, and even when there is a range, we were not careful about integer overflow (offset=5E range=5E happily claims to export a 5E image; but all bets are off if you later try to access beyond the real underlying size). And in several cases, such as if the plugin's get_size fails but range was not provided, we are not returning -1 for failure.
Fixes: 3db69f56 Signed-off-by: Eric Blake <eblake@redhat.com> ---
The first post-1.14 bugfix. Too bad I didn't spot it earlier today.
The patch looks good, ACK. We can backport these fixes easily to 1.14.1, but let's give it a few days as I'm sure there will be several more bugs that we find. However do please push this today, because I will do a final [probably] 1.12 stable branch release later today. Thanks, Rich.
filters/offset/offset.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/filters/offset/offset.c b/filters/offset/offset.c index 7df1ed13..1039a577 100644 --- a/filters/offset/offset.c +++ b/filters/offset/offset.c @@ -64,16 +64,9 @@ offset_config (nbdkit_next_config *next, void *nxdata, return next (nxdata, key, value); }
-/* Check the user did pass both parameters. */ -static int -offset_config_complete (nbdkit_next_config_complete *next, void *nxdata) -{ - return next (nxdata); -} - #define offset_config_help \ - "offset=<OFFSET> (required) The start offset to serve.\n" \ - "range=<LENGTH> The total size to serve." + "offset=<OFFSET> The start offset to serve (default 0).\n" \ + "range=<LENGTH> The total size to serve (default rest of file)."
/* Get the file size. */ static int64_t @@ -82,16 +75,23 @@ offset_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, { int64_t real_size = next_ops->get_size (nxdata);
+ if (real_size == -1) + return -1; + if (range >= 0) { - if (offset + range > real_size) { + if (offset > real_size - range) { nbdkit_error ("offset+range is larger than the real size " "of the underlying file or device"); return -1; } return range; } - else - return real_size - offset; + else if (offset > real_size) { + nbdkit_error ("offset is larger than the real size " + "of the underlying file or device"); + return -1; + } + return real_size - offset; }
/* Read data. */ @@ -176,7 +176,6 @@ static struct nbdkit_filter filter = { .longname = "nbdkit offset filter", .version = PACKAGE_VERSION, .config = offset_config, - .config_complete = offset_config_complete, .config_help = offset_config_help, .get_size = offset_get_size, .pread = offset_pread, -- 2.21.0
_______________________________________________ Libguestfs mailing list Libguestfs@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-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/