[Libguestfs] [PATCH libnbd 2/2] copy: Fix sparsifying if start is not aligned

[Richard W.M. Jones]

On Tue, Feb 23, 2021 at 03:41:26PM +0200, Nir Soffer wrote:
> When using --sparse=, we may have a case when command start is not
> aligned to sparse_size and command length is smaller than sparse_size.
> In this case we advance i after the end of the command, and abort when
> we try to write after end.
> 
> To reproduce this issue (with temporary debug logs):
> 
> $ ./nbdcopy --sparse=$((64*1024)) --threads=1 --requests=1 nbd+unix:///?socket=/tmp/src.sock null:
> sparsifying range start=0 end=4096 len=4096
> write unaligned tail offset=0 end=4096 len=4096
> sparsifying range start=1048576 end=1114112 len=65536
> write last data start=1048576 end=1114112 len=65536
> sparsifying range start=2097152 end=2121728 len=24576
> write unaligned tail offset=2097152 end=2121728 len=24576
> sparsifying range start=2138112 end=2146304 len=8192
> write last data start=2138112 end=2162688 len=24576
> lt-nbdcopy: multi-thread-copying.c:351: copy_subcommand: Assertion `offset + len <= end' failed.
> Aborted (core dumped)
> 
> Fixed by clipping initial value of i to end of the image.
> ---
>  copy/multi-thread-copying.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/copy/multi-thread-copying.c b/copy/multi-thread-copying.c
> index a1f4b19..ac917e9 100644
> --- a/copy/multi-thread-copying.c
> +++ b/copy/multi-thread-copying.c
> @@ -367,6 +367,12 @@ copy_subcommand (struct command *command, uint64_t offset, size_t len,
>    return newcommand;
>  }
>  
> +static inline uint64_t
> +min(uint64_t a, uint64_t b)
> +{
> +    return a < b ? a : b;
> +}
> +
>  /* Callback called when src has finished one read command.  This
>   * initiates a write.
>   */
> @@ -399,7 +405,7 @@ finished_read (void *vp, int *error)
>      /* Iterate over whole blocks in the command, starting on a block
>       * boundary.
>       */
> -    for (i = ROUND_UP (start, sparse_size);
> +    for (i = min (ROUND_UP (start, sparse_size), end);
>           i + sparse_size <= end;
>           i += sparse_size) {
>  

ACK, thanks.

I'll probably follow up with a patch to use common/include/minmax.h
from nbdkit.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top