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/input_file.c') 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/input_file.c') 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 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_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/input/input_file.c') 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 -- cgit v1.2.3