Libguestfs - May 2019 - [nbdkit PATCH] truncate: Detect large image overflow with round-up

It is possible for the round-up parameter to cause our desired size to
exceed 2**63-1.  But this error message is cryptic:

$ ./nbdkit -f --filter=truncate pattern $(((1<<63)-1)) round-up=1m &
$ qemu-nbd --list
nbdkit: pattern[1]: error: .get_size function returned invalid value
(-9223372036854775808)
qemu-nbd: Failed to read initial magic: Unexpected end-of-file before all bytes
were read

Better is the result of this patch:
nbdkit: pattern[1]: error: cannot round size 9223372036854775807 up to next
boundary of 1048576

Alas, we are still limited in the fact that we can only detect the
problem on a per-connection basis (at .prepare, rather than at .load)
(because we support plugins that have a different size per
connection); and that nbdkit in general is still rather rude to
clients when .prepare prevents the use of a connection (perhaps nbdkit
should instead be taught to proceed with the socekt, but respond to
NBD_OPT_LIST with 0 items and to NBD_OPT_GO with NBD_REP_ERR_UNKNOWN
or NBD_REP_ERR_SHUTDOWN to give the client a hint that this server has
nothing further to offer).

Signed-off-by: Eric Blake <eblake@redhat.com>
---

I pushed this one, but am posting to the list because of the
conversation points it brings up about what to do when .prepare fails
(there's a difference between refusing to start nbdkit because .config
failed, vs. starting but being useless to a client because
.open/.prepare failed).

 filters/truncate/truncate.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/filters/truncate/truncate.c b/filters/truncate/truncate.c
index 6408c35..bed5a03 100644
--- a/filters/truncate/truncate.c
+++ b/filters/truncate/truncate.c
@@ -38,6 +38,7 @@
 #include <string.h>
 #include <limits.h>
 #include <errno.h>
+#include <inttypes.h>

 #include <nbdkit-filter.h>

@@ -172,8 +173,14 @@ truncate_prepare (struct nbdkit_next_ops *next_ops, void
*nxdata,
    */
   if (truncate_size >= 0)
     h->size = truncate_size;
-  if (round_up > 0)
+  if (round_up > 0) {
+    if (ROUND_UP (h->size, round_up) > INT64_MAX) {
+      nbdkit_error ("cannot round size %" PRId64 " up to next
boundary of %u",
+                    h->size, round_up);
+      return -1;
+    }
     h->size = ROUND_UP (h->size, round_up);
+  }
   if (round_down > 0)
     h->size = ROUND_DOWN (h->size, round_down);

-- 
2.20.1

On Fri, May 17, 2019 at 04:05:22PM -0500, Eric Blake
wrote:
> It is possible for the round-up parameter to cause our desired size to
> exceed 2**63-1.  But this error message is cryptic:
> 
> $ ./nbdkit -f --filter=truncate pattern $(((1<<63)-1)) round-up=1m
&
> $ qemu-nbd --list
> nbdkit: pattern[1]: error: .get_size function returned invalid value
(-9223372036854775808)
> qemu-nbd: Failed to read initial magic: Unexpected end-of-file before all
bytes were read
> 
> Better is the result of this patch:
> nbdkit: pattern[1]: error: cannot round size 9223372036854775807 up to next
boundary of 1048576
> 
> Alas, we are still limited in the fact that we can only detect the
> problem on a per-connection basis (at .prepare, rather than at .load)
> (because we support plugins that have a different size per
> connection); and that nbdkit in general is still rather rude to
> clients when .prepare prevents the use of a connection (perhaps nbdkit
> should instead be taught to proceed with the socekt, but respond to
> NBD_OPT_LIST with 0 items and to NBD_OPT_GO with NBD_REP_ERR_UNKNOWN
> or NBD_REP_ERR_SHUTDOWN to give the client a hint that this server has
> nothing further to offer).
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> I pushed this one, but am posting to the list because of the
> conversation points it brings up about what to do when .prepare fails
> (there's a difference between refusing to start nbdkit because .config
> failed, vs. starting but being useless to a client because
> .open/.prepare failed).
> 
>  filters/truncate/truncate.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/filters/truncate/truncate.c b/filters/truncate/truncate.c
> index 6408c35..bed5a03 100644
> --- a/filters/truncate/truncate.c
> +++ b/filters/truncate/truncate.c
> @@ -38,6 +38,7 @@
>  #include <string.h>
>  #include <limits.h>
>  #include <errno.h>
> +#include <inttypes.h>
> 
>  #include <nbdkit-filter.h>
> 
> @@ -172,8 +173,14 @@ truncate_prepare (struct nbdkit_next_ops *next_ops,
void *nxdata,
>     */
>    if (truncate_size >= 0)
>      h->size = truncate_size;
> -  if (round_up > 0)
> +  if (round_up > 0) {
> +    if (ROUND_UP (h->size, round_up) > INT64_MAX) {
> +      nbdkit_error ("cannot round size %" PRId64 " up to
next boundary of %u",
> +                    h->size, round_up);
> +      return -1;
> +    }
>      h->size = ROUND_UP (h->size, round_up);
> +  }
>    if (round_down > 0)
>      h->size = ROUND_DOWN (h->size, round_down);
> 
> -- 
> 2.20.1
Looks sensible anyway, thanks for pushing it.

Rich.

-- 
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