[Libguestfs] [nbdkit PATCH] offset: Better handling of parameters
Richard W.M. Jones
rjones at redhat.com
Thu Aug 29 10:16:07 UTC 2019
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 at 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 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-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/
More information about the Libguestfs
mailing list