Re: [Libcdio-devel] udf_read_sectors cannot handle offsets past 2 GB

[Pete Batard]

Proposed fix for the issue above. Obviously it requires LFS support for 
64 bit off_t. This commit has already been pushed to the -pbatard branch.

Note that the attached patch has a redundant AC_FUNC_SEEKO, which is 
fixed in a later commit (had already pushed to the public repo when I 
realized the issue).

Some extra sanity checks for lba and offset values are added, as 
negative values there can be expected for 32 bit platforms without LFS. 
I also spotted a problem in a STREAM_DEBUG statement which I fixed, and 
added some beautification in struct udf_s (besides changing i_position 
from ssize_t to off_t).

With this, and provided you apply the next extra.c fix (cf. next mail), 
you should get working extraction of the Windows 8 preview image on 
Linux 32 bit, including the > 4GB /sources/install.wim file. I haven't 
methodically checked all the extract files, but the ones I did looked 
fine and the md5sum of /sources/install.wim matches the one of the same 
file extracted with 7-zip.

Regards,

/Pete

>From 845d2b3ab3bb54500b75901d536f3d8377ea7f6b Mon Sep 17 00:00:00 2001
From: Pete Batard <address@hidden>
Date: Tue, 24 Jan 2012 11:51:50 +0000
Subject: [PATCH] Fix access for stdio streams larger than 2 GB

* stdio streams relied on fseek, that only accepts a long for the offset
* switch to using of fseeko where available, that takes an off_t which
  can be 64 bit on platforms with LFS
---
 configure.ac              |    1 +
 lib/driver/_cdio_stdio.c  |   17 ++++++++++++++---
 lib/driver/_cdio_stream.c |    7 ++++---
 lib/driver/_cdio_stream.h |    6 +++---
 lib/udf/udf_file.c        |   19 ++++++++++++++-----
 lib/udf/udf_fs.c          |   15 +++++++++++----
 lib/udf/udf_private.h     |   11 +++++------
 7 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/configure.ac b/configure.ac
index 175782d..2f52916 100644
--- a/configure.ac
+++ b/configure.ac
@@ -157,6 +157,7 @@ AC_SUBST(DIFF_OPTS)
 
 dnl check for large file support
 AC_SYS_LARGEFILE
+AC_FUNC_FSEEKO
 
 dnl we need to define _FILE_OFFSET_BITS or _LARGE_FILES on the compiler command
 dnl line because otherwise the system headers risk being included before
diff --git a/lib/driver/_cdio_stdio.c b/lib/driver/_cdio_stdio.c
index 8d692d3..1a3a1eb 100644
--- a/lib/driver/_cdio_stdio.c
+++ b/lib/driver/_cdio_stdio.c
@@ -37,6 +37,17 @@
 #include "_cdio_stream.h"
 #include "_cdio_stdio.h"
 
+/* On 32 bit platforms, fseek can only access streams of 2 GB or less.
+   Prefer fseeko, which takes a 64 bit off_t when LFS is enabled */
+#ifdef HAVE_FSEEKO
+# define STDIO_SEEK fseeko
+#else
+# define STDIO_SEEK fseek
+#endif
+
+#define _STRINGIFY(a) #a
+#define STRINGIFY(a) _STRINGIFY(a)
+
 static const char _rcsid[] = "$Id: _cdio_stdio.c,v 1.6 2008/04/22 15:29:11 
karl Exp $";
 
 #define CDIO_STDIO_BUFSIZE (128*1024)
@@ -109,12 +120,12 @@ _stdio_free(void *user_data)
   indicate the error.
 */
 static driver_return_code_t 
-_stdio_seek(void *p_user_data, long i_offset, int whence)
+_stdio_seek(void *p_user_data, off_t i_offset, int whence)
 {
   _UserData *const ud = p_user_data;
 
-  if ( (i_offset=fseek (ud->fd, i_offset, whence)) ) {
-    cdio_error ("fseek (): %s", strerror (errno));
+  if ( (i_offset=STDIO_SEEK (ud->fd, i_offset, whence)) ) {
+    cdio_error ( STRINGIFY(STDIO_SEEK) " (): %s", strerror (errno));
   }
 
   return i_offset;
diff --git a/lib/driver/_cdio_stream.c b/lib/driver/_cdio_stream.c
index c1d0489..a631b1e 100644
--- a/lib/driver/_cdio_stream.c
+++ b/lib/driver/_cdio_stream.c
@@ -43,7 +43,7 @@ struct _CdioDataSource {
   void* user_data;
   cdio_stream_io_functions op;
   int is_open;
-  long position;
+  off_t position;
 };
 
 void
@@ -169,7 +169,7 @@ cdio_stream_read(CdioDataSource_t* p_obj, void *ptr, long 
size, long nmemb)
   the global variable errno is set to indicate the error.
 */
 ssize_t
-cdio_stream_seek(CdioDataSource_t* p_obj, ssize_t offset, int whence)
+cdio_stream_seek(CdioDataSource_t* p_obj, off_t offset, int whence)
 {
   if (!p_obj) return DRIVER_OP_UNINIT;
 
@@ -178,10 +178,11 @@ cdio_stream_seek(CdioDataSource_t* p_obj, ssize_t offset, 
int whence)
     return DRIVER_OP_ERROR;
 
   if (offset < 0) return DRIVER_OP_ERROR;
+  if (p_obj->position < 0) return DRIVER_OP_ERROR;
 
   if (p_obj->position != offset) {
 #ifdef STREAM_DEBUG
-    cdio_warn("had to reposition DataSource from %ld to %ld!", obj->position, 
offset);
+    cdio_warn("had to reposition DataSource from %ld to %ld!", 
p_obj->position, offset);
 #endif
     p_obj->position = offset;
     return p_obj->op.seek(p_obj->user_data, offset, whence);
diff --git a/lib/driver/_cdio_stream.h b/lib/driver/_cdio_stream.h
index 7147a0b..31f044c 100644
--- a/lib/driver/_cdio_stream.h
+++ b/lib/driver/_cdio_stream.h
@@ -35,7 +35,7 @@ extern "C" {
   
   typedef long(*cdio_data_read_t)(void *user_data, void *buf, long count);
   
-  typedef driver_return_code_t(*cdio_data_seek_t)(void *user_data, long offset,
+  typedef driver_return_code_t(*cdio_data_seek_t)(void *user_data, off_t 
offset,
                                                   int whence);
   
   typedef long(*cdio_data_stat_t)(void *user_data);
@@ -92,7 +92,7 @@ extern "C" {
                            long nmemb);
   
   /** 
-    Like fseek(3) and in fact may be the same.
+    Like fseek(3)/fseeko(3) and in fact may be the same.
 
     This  function sets the file position indicator for the stream
     pointed to by stream.  The new position, measured in bytes, is obtained
@@ -107,7 +107,7 @@ extern "C" {
     DRIVER_OP_ERROR is returned and the global variable errno is set to
     indicate the error.
    */
-  ssize_t cdio_stream_seek(CdioDataSource_t *p_obj, ssize_t i_offset, 
+  ssize_t cdio_stream_seek(CdioDataSource_t *p_obj, off_t i_offset, 
                            int whence);
   
   /**
diff --git a/lib/udf/udf_file.c b/lib/udf/udf_file.c
index ca4d920..1b3ddab 100644
--- a/lib/udf/udf_file.c
+++ b/lib/udf/udf_file.c
@@ -115,7 +115,12 @@ offset_to_lba(const udf_dirent_t *p_udf_dirent, off_t 
i_offset,
     &p_udf_dirent->fe;
   const udf_icbtag_t *p_icb_tag = &p_udf_fe->icb_tag;
   const uint16_t strat_type= uint16_from_le(p_icb_tag->strat_type);
-  
+
+  if (i_offset < 0) {
+    cdio_warn("Negative offset value");
+    return CDIO_INVALID_LBA;
+  }
+
   switch (strat_type) {
   case 4096:
     printf("Cannot deal with strategy4096 yet!\n");
@@ -123,8 +128,8 @@ offset_to_lba(const udf_dirent_t *p_udf_dirent, off_t 
i_offset,
     break;
   case ICBTAG_STRATEGY_TYPE_4:
     {
-      uint32_t icblen = 0;
-      lba_t lsector;
+      off_t icblen = 0;
+      uint64_t lsector;
       int ad_offset, ad_num = 0;
       uint16_t addr_ilk = uint16_from_le(p_icb_tag->flags&ICBTAG_FLAG_AD_MASK);
       
@@ -199,8 +204,12 @@ offset_to_lba(const udf_dirent_t *p_udf_dirent, off_t 
i_offset,
        printf("Unsupported allocation descriptor %d\n", addr_ilk);
        return CDIO_INVALID_LBA;
       }
-      
-      *pi_lba = lsector + p_udf->i_part_start;
+
+      *pi_lba = (lba_t)lsector + p_udf->i_part_start;
+      if (*pi_lba < 0) {
+       cdio_warn("Negative LBA value");
+       return CDIO_INVALID_LBA;
+      }
       return *pi_lba;
     }
   default:
diff --git a/lib/udf/udf_fs.c b/lib/udf/udf_fs.c
index eb1da77..e7df98a 100644
--- a/lib/udf/udf_fs.c
+++ b/lib/udf/udf_fs.c
@@ -312,14 +312,21 @@ udf_new_dirent(udf_file_entry_t *p_udf_fe, udf_t *p_udf,
 */
 driver_return_code_t
 udf_read_sectors (const udf_t *p_udf, void *ptr, lsn_t i_start, 
-                long int i_blocks) 
+                long i_blocks) 
 {
   driver_return_code_t ret;
-  long int i_read;
-  long int i_byte_offset;
+  long i_read;
+  off_t i_byte_offset;
   
   if (!p_udf) return 0;
-  i_byte_offset = (i_start * UDF_BLOCKSIZE);
+  /* Without the cast, i_start * UDF_BLOCKSIZE may be evaluated as 32 bit */
+  i_byte_offset = ((off_t)i_start) * UDF_BLOCKSIZE;
+  /* Since we're using SEEK_SET, the value must be positive */
+  if (i_byte_offset < 0) {
+    if (sizeof(off_t) <= 4)    /* probably missing LFS */
+      cdio_warn("Large File Support is required to access streams of 2 GB or 
more");
+    return DRIVER_OP_BAD_PARAMETER;
+  }
 
   if (p_udf->b_stream) {
     ret = cdio_stream_seek (p_udf->stream, i_byte_offset, SEEK_SET);
diff --git a/lib/udf/udf_private.h b/lib/udf/udf_private.h
index 92b1668..cfc4076 100644
--- a/lib/udf/udf_private.h
+++ b/lib/udf/udf_private.h
@@ -31,13 +31,12 @@
 /* Implementation of opaque types */
 
 struct udf_s {
-  bool          b_stream;         /* Use stream pointer, else use 
-                                   p_cdio.  */
-  ssize_t               i_position; /* Position in file if positive. */
-  CdioDataSource_t      *stream;  /* Stream pointer if stream */
-  CdIo_t                *cdio;    /* Cdio pointer if read device */
+  bool                  b_stream;     /* Use stream pointer, else use p_cdio */
+  off_t                 i_position;   /* Position in file if positive */
+  CdioDataSource_t      *stream;      /* Stream pointer if stream */
+  CdIo_t                *cdio;        /* Cdio pointer if read device */
   anchor_vol_desc_ptr_t anchor_vol_desc_ptr;
-  uint32_t              pvd_lba;  /* sector of Primary Volume Descriptor */
+  uint32_t              pvd_lba;      /* sector of Primary Volume Descriptor */
   partition_num_t       i_partition;  /* partition number */
   uint32_t              i_part_start; /* start of Partition Descriptor */
   uint32_t              lvd_lba;      /* sector of Logical Volume Descriptor */
-- 
1.7.8.msysgit.0