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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2182 bytes --]

On 2018-02-28 14:34, Daniel P. Berrangé wrote:
> 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 <berrange@redhat.com>
>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1549231
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  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) {
>       ...
>    }

Hmmm... On my system, it appears to be a long int[1].  And
find_allocation() does an off_t < 0 comparison already.  And
"man 0p sys_types.h" says "blkcnt_t and off_t shall be signed integer
types."

Max


[1]

#define do_stringify(x) #x



#define stringify(x) do_stringify(x)

int main(void)
{
    printf("%s\n", stringify(__OFF_T_TYPE));
}

Output: long int


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]