At 01:28 AM 4/12/2003, Jiri Kuthan wrote:
Jonathan,
thanks for reporting the bug. We encountered it some time ago (it is fixed the same on CVS the way you propose, by testing char_v) but I didn't update INSTALL -- shame on me and sorry about that. It's now refered from there,
s/INSTALL/ISSUES
including a patch.
-Jiri
At 10:06 PM 4/10/2003, Jonathan Lennox wrote:
I've discovered a bug in SER 0.8.10 which can segfault the server with a dereference of NULL, one time in RAND_MAX.
In the function branch_builder() in msg_translator.c, if both the parameter 'label' and the parameter 'char_v' are 0, SER will crash. This is because the code assumes that if label is 0, char_v is non-NULL, and so will attempt to call memcpy() with char_v as the source.
When branch_builder is invoked by the tm module, however, the label parameter comes from a random value assigned by h_table.c. This value is generated by rand(). As such, its value can legitimately be 0, which will happen, on average, one time in RAND_MAX.
On Linux, RAND_MAX is 2^31, so this crash is very unlikely. However, on Solaris (where I'm doing some testing), RAND_MAX is 2^15, so this crash is reasonably common for a server under heavy load. However, this is a "valid" crash in either case; this isn't just a portability issue.
(Note that RAND_MAX == 2^15 being less than TABLE_ENTRIES == 2^16 can also cause problems, according to a comment in h_table.c, though I believe only ones of efficiency, not correctness.)
The patch below works around the problem in the simplest possible way, though it isn't a correct fix. I suspect the proper solution would be a) to reverse the logic of branch_builder() to test char_v for NULL, rather than label for non-0; and b) to check with the preprocessor if RAND_MAX is less than TABLE_ENTRIES, and if so, use random() rather than rand() in modules/tm/h_table.c.
-- Jonathan Lennox lennox@cs.columbia.edu
--- ser-0.8.10.orig/msg_translator.c Mon Oct 21 15:21:50 2002 +++ ser-0.8.10/msg_translator.c Wed Apr 9 15:31:47 2003 @@ -813,6 +813,12 @@ begin++; size--; } else return 0;
if (!label && !char_v) {LOG(L_ERR, "ERROR: branch_builder: both label and char_v ""are 0\n");return 0;}/* label is set -- use it ... */ if (label) { if (int2reverse_hex( &begin, &size, label )==-1)
Serusers mailing list serusers@lists.iptel.org http://lists.iptel.org/mailman/listinfo/serusers
-- Jiri Kuthan http://iptel.org/~jiri/
Serusers mailing list serusers@lists.iptel.org http://lists.iptel.org/mailman/listinfo/serusers
-- Jiri Kuthan http://iptel.org/~jiri/