[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] block/file-posix: Fix fully preallocated tr
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] block/file-posix: Fix fully preallocated truncate |
Date: |
Wed, 28 Feb 2018 14:55:22 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
On 2018-02-28 14:53, Daniel P. Berrangé wrote:
> On Wed, Feb 28, 2018 at 02:45:49PM +0100, Max Reitz wrote:
>> 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 <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) {
>>> ...
>>> }
>>
>> 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."
>
> Hmm, that's odd then - lseek man page explicitly said it must be cast,
> which suggested to me it could be unsigned:
>
> RETURN VALUE
> Upon successful completion, lseek() returns the resulting offset loca‐
> tion as measured in bytes from the beginning of the file. On error,
> the value (off_t) -1 is returned and errno is set to indicate the
> error.
>
> CC'ing Eric for the "official" POSIX answer....
But it also says (under NOTES):
The off_t data type is a signed integer data type specified by POSIX.1.
Max
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/2] block/file-posix: Fix fully preallocated truncate, Daniel P . Berrangé, 2018/02/28
Re: [Qemu-devel] [PATCH 0/2] block/file-posix: Fix fully preallocated truncate, Eric Blake, 2018/02/28