From 5f737e10ad20e15512495ae73b547181af37702c Mon Sep 17 00:00:00 2001 From: Pete Batard Date: Fri, 20 Jan 2012 12:20:53 +0000 Subject: [PATCH] [udf] more file extraction fixes * bad offset computation ICB * file offset must be reset for each new file entry * must use 64bit fseek to access > 2 GB media * always use a 2048 fe block in udf_dirent to allow updates without overflow --- src/iso.c | 7 +--- src/libcdio/cdio/cdio_config.h | 3 -- src/libcdio/cdio/ecma_167.h | 8 +++- src/libcdio/cdio/types.h | 3 ++ src/libcdio/cdio/udf.h | 2 +- src/libcdio/driver/_cdio_stdio.c | 10 ++--- src/libcdio/driver/_cdio_stream.c | 14 +++---- src/libcdio/driver/_cdio_stream.h | 10 ++--- src/libcdio/udf/udf.c | 2 + src/libcdio/udf/udf_file.c | 8 ++-- src/libcdio/udf/udf_fs.c | 67 ++++++++++++------------------- src/rufus.rc | 12 +++--- 12 files changed, 65 insertions(+), 81 deletions(-) diff --git a/src/iso.c b/src/iso.c index f0e8f07a..d90d0cdc 100644 --- a/src/iso.c +++ b/src/iso.c @@ -86,7 +86,6 @@ static void udf_list_files(udf_t *p_udf, udf_dirent_t *p_udf_dirent, const char if ((p_udf_dirent == NULL) || (psz_path == NULL)) return; -// udf_print_file_info(p_udf_dirent, psz_path); while (udf_readdir(p_udf_dirent)) { basename = udf_get_filename(p_udf_dirent); len = 3 + strlen(psz_path) + strlen(basename) + strlen(extract_dir); @@ -101,8 +100,6 @@ uprintf("FULLPATH: %s\n", fullpath); p_udf_dirent2 = udf_opendir(p_udf_dirent); if (p_udf_dirent2 != NULL) { udf_list_files(p_udf, p_udf_dirent2, &fullpath[strlen(extract_dir)]); - } else { -// printf("do we have problem?\n"); } } else { fd = fopen(fullpath, "wb"); @@ -111,9 +108,7 @@ uprintf("FULLPATH: %s\n", fullpath); goto err; } file_len = udf_get_file_length(p_udf_dirent); -//uprintf("file len = %d\n", file_len); i_blocks = CEILING(file_len, UDF_BLOCKSIZE); -//uprintf("i_blocks = %d\n", i_blocks); for (i=0; i header file. */ #undef HAVE_GLOB_H -/* Define if you have the iconv() function. */ -#undef HAVE_ICONV - /* Define to 1 if you have the header file. */ #define HAVE_INTTYPES_H 1 diff --git a/src/libcdio/cdio/ecma_167.h b/src/libcdio/cdio/ecma_167.h index fa655e87..bfbcd732 100644 --- a/src/libcdio/cdio/ecma_167.h +++ b/src/libcdio/cdio/ecma_167.h @@ -728,7 +728,13 @@ struct udf_file_entry_s udf_Uint32_t i_extended_attr; udf_Uint32_t i_alloc_descs; // udf_Uint8_t ext_attr[0]; - udf_Uint8_t ext_attr_alloc_descs[0]; + /* This may look wasteful, but if we didn't do it here, an UDF block + would still need to be allocated for each fe before truncation. + This also allows updates without worrying about overflows, as the + specs states that a file entry cannot be larger than a block */ + // TODO: use an union with padding to UDF_BLOCKSIZE (2048) + // Also, define UDF_BLOCKSIZE in this file + udf_Uint8_t ext_attr_alloc_descs[2048-176]; } GNUC_PACKED; typedef struct udf_file_entry_s udf_file_entry_t; diff --git a/src/libcdio/cdio/types.h b/src/libcdio/cdio/types.h index a6e6167e..1e2ac844 100644 --- a/src/libcdio/cdio/types.h +++ b/src/libcdio/cdio/types.h @@ -59,6 +59,9 @@ extern "C" { #endif /* HAVE_STDINT_H */ typedef uint8_t ubyte; +#if !defined(off64_t) +typedef int64_t off64_t; +#endif /* default HP/UX macros are broken */ #if defined(__hpux__) diff --git a/src/libcdio/cdio/udf.h b/src/libcdio/cdio/udf.h index 5a790378..38607a4c 100644 --- a/src/libcdio/cdio/udf.h +++ b/src/libcdio/cdio/udf.h @@ -51,7 +51,7 @@ typedef struct udf_dirent_s { /* This field has to come last because it is variable in length. */ udf_file_entry_t fe; -} udf_dirent_t;; +} udf_dirent_t; diff --git a/src/libcdio/driver/_cdio_stdio.c b/src/libcdio/driver/_cdio_stdio.c index 015060bc..293603f8 100644 --- a/src/libcdio/driver/_cdio_stdio.c +++ b/src/libcdio/driver/_cdio_stdio.c @@ -98,7 +98,7 @@ _stdio_free(void *user_data) } /*! - Like fseek(3) and in fact may be the same. + Like fseek64(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 @@ -113,12 +113,12 @@ _stdio_free(void *user_data) DRIVER_OP_ERROR is returned and the global variable errno is set to indicate the error. */ -static driver_return_code_t -_stdio_seek(void *p_user_data, long i_offset, int whence) +static off64_t +_stdio_seek(void *p_user_data, off64_t i_offset, int whence) { - _UserData *const ud = p_user_data; + _UserData *const ud = (_UserData*)p_user_data; - if ( (i_offset=fseek (ud->fd, i_offset, whence)) ) { + if ( (i_offset=_fseeki64 (ud->fd, i_offset, whence)) ) { cdio_error ("fseek (): %s", strerror (errno)); } diff --git a/src/libcdio/driver/_cdio_stream.c b/src/libcdio/driver/_cdio_stream.c index 80c638a0..9d8b4ced 100644 --- a/src/libcdio/driver/_cdio_stream.c +++ b/src/libcdio/driver/_cdio_stream.c @@ -53,7 +53,7 @@ struct _CdioDataSource { void* user_data; cdio_stream_io_functions op; int is_open; - long position; + off64_t position; }; void @@ -90,8 +90,8 @@ cdio_stream_destroy(CdioDataSource_t *p_obj) @return unpon successful completion, return value is positive, else, the global variable errno is set to indicate the error. */ -ssize_t -cdio_stream_getpos(CdioDataSource_t* p_obj, /*out*/ ssize_t *i_offset) +off64_t +cdio_stream_getpos(CdioDataSource_t* p_obj, /*out*/ off64_t *i_offset) { if (!p_obj || !p_obj->is_open) return DRIVER_OP_UNINIT; return *i_offset = p_obj->position; @@ -178,8 +178,8 @@ cdio_stream_read(CdioDataSource_t* p_obj, void *ptr, long size, long nmemb) @return unpon successful completion, return value is positive, else, the global variable errno is set to indicate the error. */ -ssize_t -cdio_stream_seek(CdioDataSource_t* p_obj, ssize_t offset, int whence) +off64_t +cdio_stream_seek(CdioDataSource_t* p_obj, off64_t offset, int whence) { if (!p_obj) return DRIVER_OP_UNINIT; @@ -193,8 +193,8 @@ cdio_stream_seek(CdioDataSource_t* p_obj, ssize_t offset, int whence) #ifdef STREAM_DEBUG cdio_warn("had to reposition DataSource from %ld to %ld!", obj->position, offset); #endif - p_obj->position = (long)offset; - return p_obj->op.seek(p_obj->user_data, (long)offset, whence); + p_obj->position = (off64_t)offset; + return p_obj->op.seek(p_obj->user_data, offset, whence); } return 0; diff --git a/src/libcdio/driver/_cdio_stream.h b/src/libcdio/driver/_cdio_stream.h index e820cc9d..f57e2247 100644 --- a/src/libcdio/driver/_cdio_stream.h +++ b/src/libcdio/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 off64_t(*cdio_data_seek_t)(void *user_data, off64_t offset, int whence); typedef uint64_t(*cdio_data_stat_t)(void *user_data); @@ -65,8 +65,8 @@ extern "C" { @return unpon successful completion, return value is positive, else, the global variable errno is set to indicate the error. */ - ssize_t cdio_stream_getpos(CdioDataSource_t* p_obj, - /*out*/ ssize_t *i_offset); + off64_t cdio_stream_getpos(CdioDataSource_t* p_obj, + /*out*/ off64_t *i_offset); CdioDataSource_t * cdio_stream_new(void *user_data, const cdio_stream_io_functions *funcs); @@ -92,7 +92,7 @@ extern "C" { long nmemb); /** - Like fseek(3) and in fact may be the same. + Like fseek64/_fseeki64 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, + off64_t cdio_stream_seek(CdioDataSource_t *p_obj, off64_t i_offset, int whence); /** diff --git a/src/libcdio/udf/udf.c b/src/libcdio/udf/udf.c index 723229e4..548754d0 100644 --- a/src/libcdio/udf/udf.c +++ b/src/libcdio/udf/udf.c @@ -49,6 +49,8 @@ udf_get_posix_filemode(const udf_dirent_t *p_udf_dirent) udf_file_entry_t udf_fe; mode_t mode = 0; + // FIXME: what's the point of the fe dupe? This is the only + // place that seems to use udf_get_file_entry... if (udf_get_file_entry(p_udf_dirent, &udf_fe)) { uint32_t i_perms; diff --git a/src/libcdio/udf/udf_file.c b/src/libcdio/udf/udf_file.c index 476ede19..3b442443 100644 --- a/src/libcdio/udf/udf_file.c +++ b/src/libcdio/udf/udf_file.c @@ -35,7 +35,6 @@ #define GETICB(offset) \ &p_udf_fe->ext_attr_alloc_descs[offset] -// TODO: do we need to add p_udf_fe->i_extended_attr to offset here? const char * udf_get_filename(const udf_dirent_t *p_udf_dirent) @@ -45,8 +44,7 @@ udf_get_filename(const udf_dirent_t *p_udf_dirent) return p_udf_dirent->psz_name; } -/* Get UDF File Entry. However we do NOT get the variable-length extended - attributes. */ +/* Copy an UDF File Entry into a Directory Entry structure. */ bool udf_get_file_entry(const udf_dirent_t *p_udf_dirent, /*out*/ udf_file_entry_t *p_udf_fe) @@ -149,7 +147,7 @@ offset_to_lba(const udf_dirent_t *p_udf_dirent, off_t i_offset, } p_icb = (udf_short_ad_t *) GETICB( uint32_from_le(p_udf_fe->i_extended_attr) - + ad_offset ); + + ad_offset*sizeof(udf_short_ad_t*) ); icblen = p_icb->len; ad_num++; } while(i_offset >= (off_t)icblen); @@ -176,7 +174,7 @@ offset_to_lba(const udf_dirent_t *p_udf_dirent, off_t i_offset, } p_icb = (udf_long_ad_t *) GETICB( uint32_from_le(p_udf_fe->i_extended_attr) - + ad_offset ); + + ad_offset*sizeof(udf_long_ad_t*) ); icblen = p_icb->len; ad_num++; } while(i_offset >= (off_t)icblen); diff --git a/src/libcdio/udf/udf_fs.c b/src/libcdio/udf/udf_fs.c index 2c668257..7db183c1 100644 --- a/src/libcdio/udf/udf_fs.c +++ b/src/libcdio/udf/udf_fs.c @@ -60,6 +60,8 @@ #endif #include +#include +#include "cdio_assert.h" #ifndef min #define min(a,b) (((a) < (b)) ? (a) : (b)) @@ -247,14 +249,13 @@ udf_fopen(udf_dirent_t *p_udf_root, const char *psz_name) if (p_udf_root) { char tokenline[udf_MAX_PATHLEN]; char *psz_token; - + + /* file position must be reset when accessing a new file */ + p_udf_root->p_udf->i_position = 0; + strncpy(tokenline, psz_name, udf_MAX_PATHLEN); psz_token = strtok(tokenline, udf_PATH_DELIMITERS); if (psz_token) { - /*** FIXME??? udf_dirent can be variable size due to the - extended attributes and descriptors. Given that, is this - correct? - */ udf_dirent_t *p_udf_dirent = udf_new_dirent(&p_udf_root->fe, p_udf_root->p_udf, p_udf_root->psz_name, p_udf_root->b_dir, @@ -295,11 +296,8 @@ static udf_dirent_t * udf_new_dirent(udf_file_entry_t *p_udf_fe, udf_t *p_udf, const char *psz_name, bool b_dir, bool b_parent) { - const unsigned int i_alloc_size = p_udf_fe->i_alloc_descs - + p_udf_fe->i_extended_attr; - udf_dirent_t *p_udf_dirent = (udf_dirent_t *) - calloc(1, sizeof(udf_dirent_t) + i_alloc_size); + calloc(1, sizeof(udf_dirent_t)); if (!p_udf_dirent) return NULL; p_udf_dirent->psz_name = _strdup(psz_name); @@ -310,7 +308,7 @@ udf_new_dirent(udf_file_entry_t *p_udf_fe, udf_t *p_udf, p_udf_dirent->dir_left = uint64_from_le(p_udf_fe->info_len); memcpy(&(p_udf_dirent->fe), p_udf_fe, - sizeof(udf_file_entry_t) + i_alloc_size); + sizeof(udf_file_entry_t)); udf_get_lba( p_udf_fe, &(p_udf_dirent->i_loc), &(p_udf_dirent->i_loc_end) ); return p_udf_dirent; @@ -318,7 +316,8 @@ udf_new_dirent(udf_file_entry_t *p_udf_fe, udf_t *p_udf, /*! Seek to a position i_start and then read i_blocks. Number of blocks read is - returned. One normally expects the return to be equal to i_blocks. + returned. One normally expects the return to be equal to i_blocks.# + NB: 32 bit lsn_t and i_blocks means the UDF media should not be larger than 4TB */ driver_return_code_t udf_read_sectors (const udf_t *p_udf, void *ptr, lsn_t i_start, @@ -326,10 +325,11 @@ udf_read_sectors (const udf_t *p_udf, void *ptr, lsn_t i_start, { driver_return_code_t ret; ssize_t i_read; - long int i_byte_offset; + off64_t i_byte_offset; if (!p_udf) return 0; - i_byte_offset = (i_start * UDF_BLOCKSIZE); + /* i_start * UDF_BLOCKSIZE is evaluated as 32 bit by default */ + i_byte_offset = ((off64_t)i_start) * UDF_BLOCKSIZE; if (p_udf->b_stream) { ret = cdio_stream_seek (p_udf->stream, i_byte_offset, SEEK_SET); @@ -355,6 +355,9 @@ udf_open (const char *psz_path) udf_t *p_udf = (udf_t *) calloc(1, sizeof(udf_t)) ; uint8_t data[UDF_BLOCKSIZE]; + /* Sanity check */ + cdio_assert(sizeof(udf_file_entry_t) == UDF_BLOCKSIZE); + if (!p_udf) return NULL; p_udf->cdio = cdio_open(psz_path, DRIVER_UNKNOWN); @@ -592,19 +595,18 @@ udf_opendir(const udf_dirent_t *p_udf_dirent) { if (p_udf_dirent->b_dir && !p_udf_dirent->b_parent && p_udf_dirent->fid) { udf_t *p_udf = p_udf_dirent->p_udf; - uint8_t data[UDF_BLOCKSIZE]; - udf_file_entry_t *p_udf_fe = (udf_file_entry_t *) &data; + udf_file_entry_t udf_fe; driver_return_code_t i_ret = - udf_read_sectors(p_udf, p_udf_fe, p_udf->i_part_start + udf_read_sectors(p_udf, &udf_fe, p_udf->i_part_start + p_udf_dirent->fid->icb.loc.lba, 1); if (DRIVER_OP_SUCCESS == i_ret - && !udf_checktag(&p_udf_fe->tag, TAGID_FILE_ENTRY)) { + && !udf_checktag(&udf_fe.tag, TAGID_FILE_ENTRY)) { - if (ICBTAG_FILE_TYPE_DIRECTORY == p_udf_fe->icb_tag.file_type) { + if (ICBTAG_FILE_TYPE_DIRECTORY == udf_fe.icb_tag.file_type) { udf_dirent_t *p_udf_dirent_new = - udf_new_dirent(p_udf_fe, p_udf, p_udf_dirent->psz_name, true, true); + udf_new_dirent(&udf_fe, p_udf, p_udf_dirent->psz_name, true, true); return p_udf_dirent_new; } } @@ -622,7 +624,10 @@ udf_readdir(udf_dirent_t *p_udf_dirent) return NULL; } + /* file position must be reset when accessing a new file */ p_udf = p_udf_dirent->p_udf; + p_udf->i_position = 0; + if (p_udf_dirent->fid) { /* advance to next File Identifier Descriptor */ /* FIXME: need to advance file entry (fe) as well. */ @@ -665,33 +670,11 @@ udf_readdir(udf_dirent_t *p_udf_dirent) { const unsigned int i_len = p_udf_dirent->fid->i_file_id; - uint8_t data[UDF_BLOCKSIZE] = {0}; - udf_file_entry_t *p_udf_fe = (udf_file_entry_t *) &data; - udf_Uint32_t i_alloc_descs = p_udf_dirent->fe.i_alloc_descs; - udf_Uint32_t i_extended_attr = p_udf_dirent->fe.i_extended_attr; - if (DRIVER_OP_SUCCESS != udf_read_sectors(p_udf, p_udf_fe, p_udf->i_part_start + if (DRIVER_OP_SUCCESS != udf_read_sectors(p_udf, &p_udf_dirent->fe, p_udf->i_part_start + p_udf_dirent->fid->icb.loc.lba, 1)) return NULL; -/* There is a bug here, as we may use a file entry with i_alloc_descs or i_extended_attr - that doesn't match the one we used when allocating the structure. If they are bigger - memcpy will result in memory overflow and corruption. Use min() as a workaround. */ -if ((p_udf_fe->i_alloc_descs != p_udf_dirent->fe.i_alloc_descs)) { - cdio_debug("MISMATCH! p_udf_dirent = %p: i_alloc_desc %d (new LBA) vs %d (existing)", p_udf_dirent, p_udf_fe->i_alloc_descs, p_udf_dirent->fe.i_alloc_descs); - i_alloc_descs = min(p_udf_fe->i_alloc_descs, p_udf_dirent->fe.i_alloc_descs); -} -if ((p_udf_fe->i_extended_attr != p_udf_dirent->fe.i_extended_attr)) { - cdio_debug("MISMATCH! p_udf_dirent = %p: i_extended_attr %d (new LBA) vs %d (existing)", p_udf_dirent, p_udf_fe->i_extended_attr, p_udf_dirent->fe.i_extended_attr); - i_extended_attr = min(p_udf_fe->i_extended_attr, p_udf_dirent->fe.i_extended_attr); -} - - memcpy(&(p_udf_dirent->fe), p_udf_fe, - sizeof(udf_file_entry_t) + min(p_udf_fe->i_alloc_descs - + p_udf_fe->i_extended_attr, p_udf_dirent->fe.i_alloc_descs + p_udf_dirent->fe.i_extended_attr)); - p_udf_dirent->fe.i_alloc_descs = i_alloc_descs; - p_udf_dirent->fe.i_extended_attr = i_extended_attr; - if (strlen(p_udf_dirent->psz_name) < i_len) p_udf_dirent->psz_name = (char *) realloc(p_udf_dirent->psz_name, sizeof(char)*i_len+1); diff --git a/src/rufus.rc b/src/rufus.rc index da22d271..b62fca66 100644 --- a/src/rufus.rc +++ b/src/rufus.rc @@ -33,7 +33,7 @@ LANGUAGE LANG_ENGLISH, SUBLANG_NEUTRAL IDD_DIALOG DIALOGEX 12, 12, 206, 278 STYLE DS_SETFONT | DS_MODALFRAME | DS_FIXEDSYS | WS_POPUP | WS_CAPTION | WS_SYSMENU EXSTYLE WS_EX_APPWINDOW -CAPTION "Rufus v1.0.7.140" +CAPTION "Rufus v1.0.7.141" FONT 8, "MS Shell Dlg", 400, 0, 0x1 BEGIN DEFPUSHBUTTON "Start",IDC_START,94,236,50,14 @@ -70,7 +70,7 @@ BEGIN DEFPUSHBUTTON "OK",IDOK,231,175,50,14,WS_GROUP CONTROL "http://rufus.akeo.ie",IDC_ABOUT_RUFUS_URL, "SysLink",WS_TABSTOP,46,47,114,9 - LTEXT "Version 1.0.7 (Build 140)",IDC_STATIC,46,19,78,8 + LTEXT "Version 1.0.7 (Build 141)",IDC_STATIC,46,19,78,8 PUSHBUTTON "License...",IDC_ABOUT_LICENSE,46,175,50,14,WS_GROUP EDITTEXT IDC_ABOUT_COPYRIGHTS,46,107,235,63,ES_MULTILINE | ES_READONLY | WS_VSCROLL LTEXT "Report bugs or request enhancements at:",IDC_STATIC,46,66,187,8 @@ -208,8 +208,8 @@ END // VS_VERSION_INFO VERSIONINFO - FILEVERSION 1,0,7,140 - PRODUCTVERSION 1,0,7,140 + FILEVERSION 1,0,7,141 + PRODUCTVERSION 1,0,7,141 FILEFLAGSMASK 0x3fL #ifdef _DEBUG FILEFLAGS 0x1L @@ -226,13 +226,13 @@ BEGIN BEGIN VALUE "CompanyName", "akeo.ie" VALUE "FileDescription", "Rufus" - VALUE "FileVersion", "1.0.7.140" + VALUE "FileVersion", "1.0.7.141" VALUE "InternalName", "Rufus" VALUE "LegalCopyright", "© 2011 Pete Batard (GPL v3)" VALUE "LegalTrademarks", "http://www.gnu.org/copyleft/gpl.html" VALUE "OriginalFilename", "rufus.exe" VALUE "ProductName", "Rufus" - VALUE "ProductVersion", "1.0.7.140" + VALUE "ProductVersion", "1.0.7.141" END END BLOCK "VarFileInfo"