Improve lseek handling (#2564)

The skip functions are limited to 1 GB for cases in which libarchive
runs on a system with an off_t or long with 32 bits. This has negative
impact on 64 bit systems.

Instead, make sure that _all_ subsequent functions truncate properly.
Some of them already did and some had regressions for over 10 years.

Tests pass on Debian 12 i686 configured with --disable-largefile, i.e.
running with an off_t with 32 bits.

Casts added where needed to still pass MSVC builds.

---------

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>

diff --git a/libarchive/archive_read.c b/libarchive/archive_read.c
index 822c534b868a9e89318476fadd53ee379a77067e..50db87017706c565adb2813250ca7dcd5ff5c3e5 100644 (file)
--- a/libarchive/archive_read.c
+++ b/libarchive/archive_read.c
@@ -176,15 +176,9 @@ client_skip_proxy(struct archive_read_filter *self, int64_t request)
                return 0;
 
        if (self->archive->client.skipper != NULL) {
-               /* Seek requests over 1GiB are broken down into
-                * multiple seeks.  This avoids overflows when the
-                * requests get passed through 32-bit arguments. */
-               int64_t skip_limit = (int64_t)1 << 30;
                int64_t total = 0;
                for (;;) {
                        int64_t get, ask = request;
-                       if (ask > skip_limit)
-                               ask = skip_limit;
                        get = (self->archive->client.skipper)
                                (&self->archive->archive, self->data, ask);
                        total += get;

diff --git a/libarchive/archive_read_disk_posix.c b/libarchive/archive_read_disk_posix.c
index 09965eb98a2ebcd9c806ed457f2dc394f198d2b3..4839d62b291d39a1897495141ce84e04376d2184 100644 (file)
--- a/libarchive/archive_read_disk_posix.c
+++ b/libarchive/archive_read_disk_posix.c
@@ -778,7 +778,8 @@ _archive_read_data_block(struct archive *_a, const void **buff,
         */
        if (t->current_sparse->offset > t->entry_total) {
                if (lseek(t->entry_fd,
-                   (off_t)t->current_sparse->offset, SEEK_SET) < 0) {
+                   (off_t)t->current_sparse->offset, SEEK_SET) !=
+                   t->current_sparse->offset) {
                        archive_set_error(&a->archive, errno, "Seek error");
                        r = ARCHIVE_FATAL;
                        a->archive.state = ARCHIVE_STATE_FATAL;

diff --git a/libarchive/archive_read_open_fd.c b/libarchive/archive_read_open_fd.c
index debfde2086872f51de981dada9a398390a80544e..3fd536d57b1f11271aab5c5d2791a0c3cd345950 100644 (file)
--- a/libarchive/archive_read_open_fd.c
+++ b/libarchive/archive_read_open_fd.c
@@ -131,7 +131,7 @@ static int64_t
 file_skip(struct archive *a, void *client_data, int64_t request)
 {
        struct read_fd_data *mine = (struct read_fd_data *)client_data;
-       int64_t skip = request;
+       off_t skip = (off_t)request;
        int64_t old_offset, new_offset;
        int skip_bits = sizeof(skip) * 8 - 1;  /* off_t is a signed type. */
 
@@ -140,15 +140,15 @@ file_skip(struct archive *a, void *client_data, int64_t request)
 
        /* Reduce a request that would overflow the 'skip' variable. */
        if (sizeof(request) > sizeof(skip)) {
-               int64_t max_skip =
+               const int64_t max_skip =
                    (((int64_t)1 << (skip_bits - 1)) - 1) * 2 + 1;
                if (request > max_skip)
-                       skip = max_skip;
+                       skip = (off_t)max_skip;
        }
 
-       /* Reduce request to the next smallest multiple of block_size */
-       request = (request / mine->block_size) * mine->block_size;
-       if (request == 0)
+       /* Reduce 'skip' to the next smallest multiple of block_size */
+       skip = (off_t)(((int64_t)skip / mine->block_size) * mine->block_size);
+       if (skip == 0)
                return (0);
 
        if (((old_offset = lseek(mine->fd, 0, SEEK_CUR)) >= 0) &&
@@ -178,11 +178,24 @@ static int64_t
 file_seek(struct archive *a, void *client_data, int64_t request, int whence)
 {
        struct read_fd_data *mine = (struct read_fd_data *)client_data;
+       off_t seek = (off_t)request;
        int64_t r;
+       int seek_bits = sizeof(seek) * 8 - 1;  /* off_t is a signed type. */
 
        /* We use off_t here because lseek() is declared that way. */
-       /* See above for notes about when off_t is less than 64 bits. */
-       r = lseek(mine->fd, request, whence);
+
+       /* Reduce a request that would overflow the 'seek' variable. */
+       if (sizeof(request) > sizeof(seek)) {
+               const int64_t max_seek =
+                   (((int64_t)1 << (seek_bits - 1)) - 1) * 2 + 1;
+               const int64_t min_seek = ~max_seek;
+               if (request > max_seek)
+                       seek = (off_t)max_seek;
+               else if (request < min_seek)
+                       seek = (off_t)min_seek;
+       }
+
+       r = lseek(mine->fd, seek, whence);
        if (r >= 0)
                return r;
 

diff --git a/libarchive/archive_read_open_file.c b/libarchive/archive_read_open_file.c
index ecd56dce618dcde7204c3c5d487fb938436ca1ba..2829b9a5c744fbf2f681e03c93585c61d4a861fa 100644 (file)
--- a/libarchive/archive_read_open_file.c
+++ b/libarchive/archive_read_open_file.c
@@ -145,7 +145,7 @@ FILE_skip(struct archive *a, void *client_data, int64_t request)
 
        /* If request is too big for a long or an off_t, reduce it. */
        if (sizeof(request) > sizeof(skip)) {
-               int64_t max_skip =
+               const int64_t max_skip =
                    (((int64_t)1 << (skip_bits - 1)) - 1) * 2 + 1;
                if (request > max_skip)
                        skip = max_skip;
@@ -176,39 +176,42 @@ FILE_seek(struct archive *a, void *client_data, int64_t request, int whence)
 {
        struct read_FILE_data *mine = (struct read_FILE_data *)client_data;
 #if HAVE__FSEEKI64
-       int64_t skip = request;
+       int64_t seek = request;
 #elif HAVE_FSEEKO
-       off_t skip = (off_t)request;
+       off_t seek = (off_t)request;
 #else
-       long skip = (long)request;
+       long seek = (long)request;
 #endif
-       int skip_bits = sizeof(skip) * 8 - 1;
+       int seek_bits = sizeof(seek) * 8 - 1;
        (void)a; /* UNUSED */
 
-       /* If request is too big for a long or an off_t, reduce it. */
-       if (sizeof(request) > sizeof(skip)) {
-               int64_t max_skip =
-                   (((int64_t)1 << (skip_bits - 1)) - 1) * 2 + 1;
-               if (request > max_skip)
-                       skip = max_skip;
+       /* Reduce a request that would overflow the 'seek' variable. */
+       if (sizeof(request) > sizeof(seek)) {
+               const int64_t max_seek =
+                   (((int64_t)1 << (seek_bits - 1)) - 1) * 2 + 1;
+               const int64_t min_seek = ~max_seek;
+               if (request > max_seek)
+                       seek = max_seek;
+               else if (request < min_seek)
+                       seek = min_seek;
        }
 
 #ifdef __ANDROID__
        /* Newer Android versions have fseeko...to meditate. */
-       int64_t ret = lseek(fileno(mine->f), skip, whence);
+       int64_t ret = lseek(fileno(mine->f), seek, whence);
        if (ret >= 0) {
                return ret;
        }
 #elif HAVE__FSEEKI64
-       if (_fseeki64(mine->f, skip, whence) == 0) {
+       if (_fseeki64(mine->f, seek, whence) == 0) {
                return _ftelli64(mine->f);
        }
 #elif HAVE_FSEEKO
-       if (fseeko(mine->f, skip, whence) == 0) {
+       if (fseeko(mine->f, seek, whence) == 0) {
                return ftello(mine->f);
        }
 #else
-       if (fseek(mine->f, skip, whence) == 0) {
+       if (fseek(mine->f, seek, whence) == 0) {
                return ftell(mine->f);
        }
 #endif
@@ -226,4 +229,4 @@ FILE_close(struct archive *a, void *client_data)
        free(mine->buffer);
        free(mine);
        return (ARCHIVE_OK);
-}
\ No newline at end of file
+}

diff --git a/libarchive/archive_read_open_filename.c b/libarchive/archive_read_open_filename.c
index 05f0ffbd9413649c2d1622362d846f45f36e2e74..3894b15c86df2a887befdbb5c5547e8e7d9a410b 100644 (file)
--- a/libarchive/archive_read_open_filename.c
+++ b/libarchive/archive_read_open_filename.c
@@ -479,20 +479,24 @@ file_skip_lseek(struct archive *a, void *client_data, int64_t request)
        struct read_file_data *mine = (struct read_file_data *)client_data;
 #if defined(_WIN32) && !defined(__CYGWIN__)
        /* We use _lseeki64() on Windows. */
-       int64_t old_offset, new_offset;
+       int64_t old_offset, new_offset, skip = request;
 #else
-       off_t old_offset, new_offset;
+       off_t old_offset, new_offset, skip = (off_t)request;
 #endif
+       int skip_bits = sizeof(skip) * 8 - 1;
 
        /* We use off_t here because lseek() is declared that way. */
 
-       /* TODO: Deal with case where off_t isn't 64 bits.
-        * This shouldn't be a problem on Linux or other POSIX
-        * systems, since the configuration logic for libarchive
-        * tries to obtain a 64-bit off_t.
-        */
+       /* Reduce a request that would overflow the 'skip' variable. */
+       if (sizeof(request) > sizeof(skip)) {
+               const int64_t max_skip =
+                   (((int64_t)1 << (skip_bits - 1)) - 1) * 2 + 1;
+               if (request > max_skip)
+                       skip = max_skip;
+       }
+
        if ((old_offset = lseek(mine->fd, 0, SEEK_CUR)) >= 0 &&
-           (new_offset = lseek(mine->fd, request, SEEK_CUR)) >= 0)
+           (new_offset = lseek(mine->fd, skip, SEEK_CUR)) >= 0)
                return (new_offset - old_offset);
 
        /* If lseek() fails, don't bother trying again. */
@@ -540,11 +544,24 @@ static int64_t
 file_seek(struct archive *a, void *client_data, int64_t request, int whence)
 {
        struct read_file_data *mine = (struct read_file_data *)client_data;
+       off_t seek = (off_t)request;
        int64_t r;
+       int seek_bits = sizeof(seek) * 8 - 1;
 
        /* We use off_t here because lseek() is declared that way. */
-       /* See above for notes about when off_t is less than 64 bits. */
-       r = lseek(mine->fd, request, whence);
+
+       /* Reduce a request that would overflow the 'seek' variable. */
+       if (sizeof(request) > sizeof(seek)) {
+               const int64_t max_seek =
+                   (((int64_t)1 << (seek_bits - 1)) - 1) * 2 + 1;
+               const int64_t min_seek = ~max_seek;
+               if (request > max_seek)
+                       seek = (off_t)max_seek;
+               else if (request < min_seek)
+                       seek = (off_t)min_seek;
+       }
+
+       r = lseek(mine->fd, seek, whence);
        if (r >= 0)
                return r;
 

diff --git a/libarchive/test/read_open_memory.c b/libarchive/test/read_open_memory.c
index 6d2468cd10a6857e6b67759356f2af5b87a2c915..9262ab9d30b313859065f3ab942be055e0ddbcc8 100644 (file)
--- a/libarchive/test/read_open_memory.c
+++ b/libarchive/test/read_open_memory.c
@@ -167,7 +167,7 @@ memory_read_skip(struct archive *a, void *client_data, int64_t skip)
 
        (void)a; /* UNUSED */
        /* We can't skip by more than is available. */
-       if ((off_t)skip > (off_t)(mine->end - mine->p))
+       if (skip > mine->end - mine->p)
                skip = mine->end - mine->p;
        /* Always do small skips by prime amounts. */
        if (skip > 71)

diff --git a/libarchive/test/test_sparse_basic.c b/libarchive/test/test_sparse_basic.c
index 23cde567059f60b4c8b2c4662c4c4eb152160978..93710cb64033cd475f7f7ebaf6954cb61181390a 100644 (file)
--- a/libarchive/test/test_sparse_basic.c
+++ b/libarchive/test/test_sparse_basic.c
@@ -608,7 +608,8 @@ DEFINE_TEST(test_sparse_basic)
        verify_sparse_file(a, "file2", sparse_file2, 20);
        /* Encoded non sparse; expect a data block but no sparse entries. */
        verify_sparse_file(a, "file3", sparse_file3, 0);
-       verify_sparse_file(a, "file4", sparse_file4, 2);
+       if (sizeof(off_t) > 4)
+               verify_sparse_file(a, "file4", sparse_file4, 2);
 
        assertEqualInt(ARCHIVE_OK, archive_read_free(a));
 
@@ -635,7 +636,8 @@ DEFINE_TEST(test_sparse_basic)
        verify_sparse_file(a, "file1", sparse_file1, 0);
        verify_sparse_file(a, "file2", sparse_file2, 0);
        verify_sparse_file(a, "file3", sparse_file3, 0);
-       verify_sparse_file(a, "file4", sparse_file4, 0);
+       if (sizeof(off_t) > 4)
+               verify_sparse_file(a, "file4", sparse_file4, 0);
 
        assertEqualInt(ARCHIVE_OK, archive_read_free(a));
 

diff --git a/libarchive/test/test_tar_large.c b/libarchive/test/test_tar_large.c
index c1f379162ad770a8b68189179b8c62bf4652f2bb..1cde32180156d618a57076871fa70d5a802e6930 100644 (file)
--- a/libarchive/test/test_tar_large.c
+++ b/libarchive/test/test_tar_large.c
@@ -175,7 +175,7 @@ memory_read_skip(struct archive *a, void *_private, int64_t skip)
        }
        if (private->filebytes > 0) {
                if (private->filebytes < skip)
-                       skip = (off_t)private->filebytes;
+                       skip = private->filebytes;
                private->filebytes -= skip;
        } else {
                skip = 0;