From a3caa5974f860b9702089c4ffea54e22f658fb49 Mon Sep 17 00:00:00 2001 From: Andrew de Quincey Date: Sun, 3 Jun 2007 02:43:07 +0100 Subject: [patch] Nasty mmap problem with huge files Hi, I've been tracking down a very odd bug this afternoon. As it turns out it is caused by enabling xine's mmap() support for the input_file.c. I'm running 32 bit linux 2.6.21. The file in question is 0x10e4da000 bytes long (you can probably guess what kind of bug this is by now :) Anyway, the issue stems from the definition of mmap(): void *mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset); compare this to the definition of st_size in struct stat: off_t st_size; /* total size, in bytes */ On my machine (in input_file.c) sizeof(size_t) ==4, whilst sizeof(off_t) == 8. However the compiler doesn't generate a warning when the following is done in xine's code: if ( (this->mmap_base = mmap(NULL, sbuf.st_size, PROT_READ, MAP_SHARED, this->fh, 0)) != (void*)-1 So it silently truncates the upper part of the length. Obviously you cannot mmap() a file that large into (32 bit) memory anyway, but as it turns out, mmapping() 0xe4da000 succeeds, which causes... problems. The patch (against xine-lib 1.1.6) does two things: * Check that the length will not be truncated, while still allowing for mmap()s of large files under 64 bit OSes. * A correctness fix: if mmap() fails, this->mmap_base will be set to 0xffffffff. Later on when the file is closed, this means it was attempting to do munmap(0xffffffff). --- src/input/input_file.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'src/input') diff --git a/src/input/input_file.c b/src/input/input_file.c index fd2b0e733..d57eaacb9 100644 --- a/src/input/input_file.c +++ b/src/input/input_file.c @@ -359,6 +359,9 @@ static int file_plugin_open (input_plugin_t *this_gen ) { file_input_plugin_t *this = (file_input_plugin_t *) this_gen; char *filename; struct stat sbuf; +#ifdef HAVE_MMAP + size_t tmp_size; +#endif lprintf("file_plugin_open\n"); @@ -423,10 +426,14 @@ static int file_plugin_open (input_plugin_t *this_gen ) { } #ifdef HAVE_MMAP - if ( (this->mmap_base = mmap(NULL, sbuf.st_size, PROT_READ, MAP_SHARED, this->fh, 0)) != (void*)-1 ) { + tmp_size = sbuf.st_size; + if ((tmp_size == sbuf.st_size) && + ( (this->mmap_base = mmap(NULL, tmp_size, PROT_READ, MAP_SHARED, this->fh, 0)) != (void*)-1 )) { this->mmap_on = 1; this->mmap_curr = this->mmap_base; this->mmap_len = sbuf.st_size; + } else { + this->mmap_base = NULL; } #endif -- cgit v1.2.3 From f398c68b695ee3dad829d95a724573ca42926205 Mon Sep 17 00:00:00 2001 From: Darren Salt Date: Sun, 3 Jun 2007 03:01:31 +0100 Subject: Add a comment & changelog entry for the mmap bug fix. --- src/input/input_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/input') diff --git a/src/input/input_file.c b/src/input/input_file.c index d57eaacb9..c755a84a8 100644 --- a/src/input/input_file.c +++ b/src/input/input_file.c @@ -426,7 +426,7 @@ static int file_plugin_open (input_plugin_t *this_gen ) { } #ifdef HAVE_MMAP - tmp_size = sbuf.st_size; + tmp_size = sbuf.st_size; /* may cause truncation - if it does, DON'T mmap! */ if ((tmp_size == sbuf.st_size) && ( (this->mmap_base = mmap(NULL, tmp_size, PROT_READ, MAP_SHARED, this->fh, 0)) != (void*)-1 )) { this->mmap_on = 1; -- cgit v1.2.3 From 72c64cd5c76febda45d95452276e11d60477103e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diego=20=27Flameeyes=27=20Petten=C3=B2?= Date: Sat, 9 Jun 2007 11:23:10 +0200 Subject: Make the read() function of input plugins be declared with void pointer as buf parameter, so that any kind of variable can be passed through it. --- src/input/input_plugin.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/input') diff --git a/src/input/input_plugin.h b/src/input/input_plugin.h index 11e1303e7..66b3abbb9 100644 --- a/src/input/input_plugin.h +++ b/src/input/input_plugin.h @@ -118,7 +118,7 @@ struct input_plugin_s { * Should block until some bytes available for read; * a return value of 0 indicates no data available */ - off_t (*read) (input_plugin_t *this, char *buf, off_t nlen); + off_t (*read) (input_plugin_t *this, void *buf, off_t nlen); /* -- cgit v1.2.3 From 4d7e574abefe9cd15c04cf0145faf55c9e282e83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diego=20=27Flameeyes=27=20Petten=C3=B2?= Date: Sat, 9 Jun 2007 11:44:00 +0200 Subject: Convert all input plugins to use void * as type for the buf parameter of read() function, and declare a new buf variable in the function as needed. --- src/input/input_cdda.c | 2 +- src/input/input_dvb.c | 4 +++- src/input/input_dvd.c | 3 ++- src/input/input_file.c | 2 +- src/input/input_gnome_vfs.c | 3 ++- src/input/input_http.c | 3 ++- src/input/input_mms.c | 3 ++- src/input/input_net.c | 3 ++- src/input/input_pnm.c | 3 ++- src/input/input_pvr.c | 3 ++- src/input/input_rtp.c | 3 ++- src/input/input_rtsp.c | 2 +- src/input/input_smb.c | 3 ++- src/input/input_stdin_fifo.c | 3 ++- src/input/input_v4l.c | 2 +- src/input/input_vcd.c | 9 ++++++--- 16 files changed, 33 insertions(+), 18 deletions(-) (limited to 'src/input') diff --git a/src/input/input_cdda.c b/src/input/input_cdda.c index b85bc534e..e35ee5b9c 100644 --- a/src/input/input_cdda.c +++ b/src/input/input_cdda.c @@ -2243,7 +2243,7 @@ static uint32_t cdda_plugin_get_capabilities (input_plugin_t *this_gen) { } -static off_t cdda_plugin_read (input_plugin_t *this_gen, char *buf, off_t len) { +static off_t cdda_plugin_read (input_plugin_t *this_gen, void *buf, off_t len) { /* only allow reading in block-sized chunks */ diff --git a/src/input/input_dvb.c b/src/input/input_dvb.c index 50162c7db..38ad0be82 100644 --- a/src/input/input_dvb.c +++ b/src/input/input_dvb.c @@ -2468,8 +2468,10 @@ static void ts_rewrite_packets (dvb_input_plugin_t *this, unsigned char * origin } static off_t dvb_plugin_read (input_plugin_t *this_gen, - char *buf, off_t len) { + void *buf_gen, off_t len) { dvb_input_plugin_t *this = (dvb_input_plugin_t *) this_gen; + char *buf = (char *)buf_gen; + off_t n=0, total=0; int have_mutex=0; struct pollfd pfd; diff --git a/src/input/input_dvd.c b/src/input/input_dvd.c index 1a8861289..dc38ceecd 100644 --- a/src/input/input_dvd.c +++ b/src/input/input_dvd.c @@ -853,8 +853,9 @@ static buf_element_t *dvd_plugin_read_block (input_plugin_t *this_gen, return buf; } -static off_t dvd_plugin_read (input_plugin_t *this_gen, char *ch_buf, off_t len) { +static off_t dvd_plugin_read (input_plugin_t *this_gen, void *buf_gen, off_t len) { /* dvd_input_plugin_t *this = (dvd_input_plugin_t*)this_gen; */ + char *ch_buf = (char *)buf_gen; /* FIXME: Tricking the demux_mpeg_block plugin */ ch_buf[0] = 0; diff --git a/src/input/input_file.c b/src/input/input_file.c index 1dce8baad..cc1e55c87 100644 --- a/src/input/input_file.c +++ b/src/input/input_file.c @@ -144,7 +144,7 @@ static int check_mmap_file(file_input_plugin_t *this) { } #endif -static off_t file_plugin_read (input_plugin_t *this_gen, char *buf, off_t len) { +static off_t file_plugin_read (input_plugin_t *this_gen, void *buf, off_t len) { file_input_plugin_t *this = (file_input_plugin_t *) this_gen; #ifdef HAVE_MMAP diff --git a/src/input/input_gnome_vfs.c b/src/input/input_gnome_vfs.c index 540abd0f4..1fe29fbcd 100644 --- a/src/input/input_gnome_vfs.c +++ b/src/input/input_gnome_vfs.c @@ -75,8 +75,9 @@ gnomevfs_plugin_get_capabilities (input_plugin_t *this_gen) #define SSH_BUFFER_SIZE 256 * 1024 static off_t -gnomevfs_plugin_read (input_plugin_t *this_gen, char *buf, off_t len) +gnomevfs_plugin_read (input_plugin_t *this_gen, void *buf_gen, off_t len) { + char *buf = (char *)buf_gen; gnomevfs_input_t *this = (gnomevfs_input_t *) this_gen; off_t n, num_bytes; diff --git a/src/input/input_http.c b/src/input/input_http.c index d1202ae14..3db5af002 100644 --- a/src/input/input_http.c +++ b/src/input/input_http.c @@ -404,8 +404,9 @@ error: } static off_t http_plugin_read (input_plugin_t *this_gen, - char *buf, off_t nlen) { + void *buf_gen, off_t nlen) { http_input_plugin_t *this = (http_input_plugin_t *) this_gen; + char *buf = (char *)buf_gen; off_t n, num_bytes; num_bytes = 0; diff --git a/src/input/input_mms.c b/src/input/input_mms.c index 739d81a59..23102ac18 100644 --- a/src/input/input_mms.c +++ b/src/input/input_mms.c @@ -98,8 +98,9 @@ typedef struct { } mms_input_class_t; static off_t mms_plugin_read (input_plugin_t *this_gen, - char *buf, off_t len) { + void *buf_gen, off_t len) { mms_input_plugin_t *this = (mms_input_plugin_t *) this_gen; + char *buf = (char *)buf_gen; off_t n = 0; lprintf ("mms_plugin_read: %"PRId64" bytes ...\n", len); diff --git a/src/input/input_net.c b/src/input/input_net.c index 0ce2e1340..dd318c37c 100644 --- a/src/input/input_net.c +++ b/src/input/input_net.c @@ -249,8 +249,9 @@ static int host_connect(const char *host, int port, xine_t *xine) { #define HIGH_WATER_MARK 100 static off_t net_plugin_read (input_plugin_t *this_gen, - char *buf, off_t len) { + void *buf_gen, off_t len) { net_input_plugin_t *this = (net_input_plugin_t *) this_gen; + char *buf = (char *)buf_gen; off_t n, total; lprintf("reading %" PRIdMAX " bytes...\n", (intmax_t)len); diff --git a/src/input/input_pnm.c b/src/input/input_pnm.c index e1413b0f7..29d65da2d 100644 --- a/src/input/input_pnm.c +++ b/src/input/input_pnm.c @@ -76,8 +76,9 @@ typedef struct { static off_t pnm_plugin_read (input_plugin_t *this_gen, - char *buf, off_t len) { + void *buf_gen, off_t len) { pnm_input_plugin_t *this = (pnm_input_plugin_t *) this_gen; + char *buf = (char *)buf_gen; off_t n; lprintf ("pnm_plugin_read: %"PRId64" bytes ...\n", len); diff --git a/src/input/input_pvr.c b/src/input/input_pvr.c index bcf93af2b..48b69c8f5 100644 --- a/src/input/input_pvr.c +++ b/src/input/input_pvr.c @@ -423,8 +423,9 @@ static uint32_t pvr_plugin_get_capabilities (input_plugin_t *this_gen) { } -static off_t pvr_plugin_read (input_plugin_t *this_gen, char *buf, off_t len) { +static off_t pvr_plugin_read (input_plugin_t *this_gen, void *buf_gen, off_t len) { /*pvr_input_plugin_t *this = (pvr_input_plugin_t *) this_gen;*/ + char *buf = (char *)buf_gen; /* FIXME: Tricking the demux_mpeg_block plugin */ buf[0] = 0; diff --git a/src/input/input_rtp.c b/src/input/input_rtp.c index d4ba804c6..2c6c581c2 100644 --- a/src/input/input_rtp.c +++ b/src/input/input_rtp.c @@ -432,8 +432,9 @@ static void * input_plugin_read_loop(void *arg) { /* ***************************************************************** */ static off_t rtp_plugin_read (input_plugin_t *this_gen, - char *buf, off_t length) { + void *buf_gen, off_t length) { rtp_input_plugin_t *this = (rtp_input_plugin_t *) this_gen; + char *buf = (char *)buf_gen; struct timeval tv; struct timespec timeout; diff --git a/src/input/input_rtsp.c b/src/input/input_rtsp.c index 693e8af66..cd2209baa 100644 --- a/src/input/input_rtsp.c +++ b/src/input/input_rtsp.c @@ -77,7 +77,7 @@ typedef struct { static off_t rtsp_plugin_read (input_plugin_t *this_gen, - char *buf, off_t len) { + void *buf, off_t len) { rtsp_input_plugin_t *this = (rtsp_input_plugin_t *) this_gen; off_t n; diff --git a/src/input/input_smb.c b/src/input/input_smb.c index 4cacebf89..87f2a81fa 100644 --- a/src/input/input_smb.c +++ b/src/input/input_smb.c @@ -66,9 +66,10 @@ smb_plugin_get_capabilities (input_plugin_t *this_gen) static off_t -smb_plugin_read (input_plugin_t *this_gen, char *buf, off_t len) +smb_plugin_read (input_plugin_t *this_gen, void *buf_gen, off_t len) { smb_input_t *this = (smb_input_t *) this_gen; + char *buf = (char *)buf_gen; off_t n, num_bytes; num_bytes = 0; diff --git a/src/input/input_stdin_fifo.c b/src/input/input_stdin_fifo.c index 939f56f25..2b3cf1376 100644 --- a/src/input/input_stdin_fifo.c +++ b/src/input/input_stdin_fifo.c @@ -81,9 +81,10 @@ static off_t stdin_plugin_get_current_pos (input_plugin_t *this_gen); static off_t stdin_plugin_read (input_plugin_t *this_gen, - char *buf, off_t len) { + void *buf_gen, off_t len) { stdin_input_plugin_t *this = (stdin_input_plugin_t *) this_gen; + char *buf = (char *)buf_gen; off_t n, total; lprintf ("reading %"PRId64" bytes...\n", len); diff --git a/src/input/input_v4l.c b/src/input/input_v4l.c index 6829470ff..959874b14 100644 --- a/src/input/input_v4l.c +++ b/src/input/input_v4l.c @@ -1192,7 +1192,7 @@ static int v4l_adjust_realtime_speed(v4l_input_plugin_t *this, fifo_buffer_t *fi * Plugin read. * This function is not supported by the plugin. */ -static off_t v4l_plugin_read (input_plugin_t *this_gen, char *buf, off_t len) { +static off_t v4l_plugin_read (input_plugin_t *this_gen, void *buf, off_t len) { lprintf("Read not supported\n"); return 0; } diff --git a/src/input/input_vcd.c b/src/input/input_vcd.c index bcd50ecc1..7d2ea0063 100644 --- a/src/input/input_vcd.c +++ b/src/input/input_vcd.c @@ -340,9 +340,10 @@ static int sun_vcd_read(vcd_input_plugin_t *this, long lba, cdsector_t *data) #if defined (__linux__) static off_t vcd_plugin_read (input_plugin_t *this_gen, - char *buf, off_t nlen) { + void *buf_gen, off_t nlen) { vcd_input_plugin_t *this = (vcd_input_plugin_t *) this_gen; + char *buf = (char *)buf_gen; static struct cdrom_msf msf ; static cdsector_t data; struct cdrom_msf0 *end_msf; @@ -398,8 +399,9 @@ static off_t vcd_plugin_read (input_plugin_t *this_gen, } #elif defined (__FreeBSD__) static off_t vcd_plugin_read (input_plugin_t *this_gen, - char *buf, off_t nlen) { + void *buf_gen, off_t nlen) { vcd_input_plugin_t *this = (vcd_input_plugin_t *) this_gen; + char *buf = (char *)buf_gen; static cdsector_t data; int bsize = 2352; @@ -422,9 +424,10 @@ static off_t vcd_plugin_read (input_plugin_t *this_gen, } #elif defined (__sun) static off_t vcd_plugin_read (input_plugin_t *this_gen, - char *buf, off_t nlen) { + void *buf_gen, off_t nlen) { vcd_input_plugin_t *this = (vcd_input_plugin_t *) this_gen; + char *buf = (char *)buf_gen; static cdsector_t data; struct cdrom_msf0 *end_msf; long lba; -- cgit v1.2.3 From 3a399036b466bbfc1ad3c379e0cc1c7d2f05e91c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diego=20=27Flameeyes=27=20Petten=C3=B2?= Date: Sat, 9 Jun 2007 11:47:30 +0200 Subject: Remove unused variable. --- src/input/input_cdda.c | 1 - 1 file changed, 1 deletion(-) (limited to 'src/input') diff --git a/src/input/input_cdda.c b/src/input/input_cdda.c index e35ee5b9c..029658fe1 100644 --- a/src/input/input_cdda.c +++ b/src/input/input_cdda.c @@ -1566,7 +1566,6 @@ static int _cdda_load_cached_cddb_infos(cdda_input_plugin_t *this) { */ static void _cdda_save_cached_cddb_infos(cdda_input_plugin_t *this, char *filecontent) { FILE *fd; - DIR *dir; char *cfile; const char *const xdg_cache_home = xdgCacheHome(this->stream->xine->basedir_handle); -- cgit v1.2.3