| Subject: | Attempt to work with a partially corrupted sharefile leads to a SIGSEGV |
Module: Cache-FastMmap-1.09.
Perl version: v5.8.5 built for sun4-solaris
OS version: SunOS 5.8 Generic_108528-19 sun4u sparc SUNW,UltraAX-i2
Problem description:
An attempt to work with a partially corrupted sharefile crashes the
process.
As far as I could understand during debug of the process, the reason for
crash is an uninitialised "last_error" variable of the "mmap_cache"
structure.
In my case the crash happens after the mmc_lock() function has revealed
the corrupted state of some page in the file.
The function returns an error condition (-1) without calling the
internal _mmc_set_error() function.
The _mmc_set_error(), if called, should store the error message in the
"last_error" variable of the "mmap_cache" structure. If it doesn't, the
uninitialized value (which points somewhere beyond the process address
space) is used as an input for croak() which leads to a crash.
Proposed patch:
The least proposed patch is to initialise the "last_error" variable in
the mmc_init() function with some constant string value like "unknown
error" or something.
Additionally, it will be a good idea to enrich error code returns from
mmc_lock() function with corresponding _mmc_set_error() calls.
A scratch patch file is included in the attachment.
| Subject: | Cache-FastMmap-1.09-patch.txt |
--- Cache-FastMmap-CImpl/mmap_cache.c.orig 2006-01-29 00:52:23.099017000 +0300
+++ Cache-FastMmap-CImpl/mmap_cache.c 2006-01-29 01:08:36.075541000 +0300
@@ -83,7 +83,7 @@
/* Internal functions */
-void _mmc_set_error(mmap_cache *, int, char *, ...);
+int _mmc_set_error(mmap_cache *, int, char *, ...);
void _mmc_init_page(mmap_cache *, MU32);
MU32 * _mmc_find_slot(mmap_cache * , MU32 , void *, int );
@@ -222,6 +222,8 @@
return -1;
}
+ cache->last_error = "Unknown error";
+
/* Basic cache params */
c_num_pages = cache->c_num_pages;
ASSERT(c_num_pages >= 1 && c_num_pages <= 1000);
@@ -449,7 +451,8 @@
return -1;
}
- if (!(P_Magic(p_ptr) == 0x92f7e3b1)) return -1;
+ if (!(P_Magic(p_ptr) == 0x92f7e3b1))
+ return -1 + _mmc_set_error( cache, 0, "magic page start marker not found. p_cur is %u, offset is %u", p_cur, p_offset);
/* Copy to cache structure */
cache->p_num_slots = P_NumSlots(p_ptr);
@@ -459,10 +462,10 @@
cache->p_free_bytes = P_FreeBytes(p_ptr);
/* Reality check */
- if (!(cache->p_num_slots >= 89 && cache->p_num_slots < cache->c_page_size)) return -1;
- if (!(cache->p_free_slots > 0 && cache->p_free_slots <= cache->p_num_slots)) return -1;
- if (!(cache->p_old_slots <= cache->p_free_slots)) return -1;
- if (!(cache->p_free_data + cache->p_free_bytes == cache->c_page_size)) return -1;
+ if (!(cache->p_num_slots >= 89 && cache->p_num_slots < cache->c_page_size)) return -1 + _mmc_set_error( cache, 0, "cache num_slots mistmatch");
+ if (!(cache->p_free_slots > 0 && cache->p_free_slots <= cache->p_num_slots)) return -1 + _mmc_set_error( cache, 0, "cache free slots mustmatch");
+ if (!(cache->p_old_slots <= cache->p_free_slots)) return -1 + _mmc_set_error( cache, 0, "cache old slots mistmatch");
+ if (!(cache->p_free_data + cache->p_free_bytes == cache->c_page_size)) return -1 + _mmc_set_error( cache, 0, "cache free data mistmatch");
/* Check page header */
ASSERT(P_Magic(p_ptr) == 0x92f7e3b1);
@@ -1228,12 +1231,12 @@
}
/*
- * void _mmc_set_error(mmap_cache *cache, int err, char * error_string, ...)
+ * int _mmc_set_error(mmap_cache *cache, int err, char * error_string, ...)
*
* Set internal error string/state
*
*/
-void _mmc_set_error(mmap_cache *cache, int err, char * error_string, ...) {
+int _mmc_set_error(mmap_cache *cache, int err, char * error_string, ...) {
va_list ap;
static char errbuf[1024];
@@ -1256,7 +1259,7 @@
va_end(ap);
- return;
+ return 0;
}
/*