Re: [Qemu-devel] [PATCH 1/2] block/file-posix: Fix fully preallocated truncate

[Daniel P . Berrangé]

On Wed, Feb 28, 2018 at 02:13:14PM +0100, Max Reitz wrote:
> Storing the lseek() result in an int results in it overflowing when the
> file is at least 2 GB big.  Then, we have a 50 % chance of the result
> being "negative" and thus thinking an error occurred when actually
> everything went just fine.
> 
> So we should use the correct type for storing the result: off_t.
> 
> Reported-by: Daniel P. Berrange <address@hidden>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1549231
> Cc: address@hidden
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  block/file-posix.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index f1591c3849..90c25864a0 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1697,6 +1697,7 @@ static int raw_regular_truncate(int fd, int64_t offset, 
> PreallocMode prealloc,
>      case PREALLOC_MODE_FULL:
>      {
>          int64_t num = 0, left = offset - current_length;
> +        off_t seek_result;
>  
>          /*
>           * Knowing the final size from the beginning could allow the file
> @@ -1711,8 +1712,8 @@ static int raw_regular_truncate(int fd, int64_t offset, 
> PreallocMode prealloc,
>  
>          buf = g_malloc0(65536);
>  
> -        result = lseek(fd, current_length, SEEK_SET);
> -        if (result < 0) {
> +        seek_result = lseek(fd, current_length, SEEK_SET);
> +        if (seek_result < 0) {

off_t is an unsigned type, so this comparison to "< 0" is bogus - only the
exact value (off_t)-1 indicates an error. So this needs to be

   if (seek_result == (off_t)-1) {
      ...
   }


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|