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 <berra...@redhat.com>
> >> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1549231
> >> Cc: qemu-sta...@nongnu.org
> >> Signed-off-by: Max Reitz <mre...@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."
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....
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 :|
