View Issue Details

IDProjectCategoryView StatusLast Update
0000463LDMud 3.3Runtimepublic2018-01-29 21:57
ReporterGnomi Assigned Tolars 
PrioritynormalSeveritycrashReproducibilityrandom
Status closedResolutionfixed 
Platformi686OSDebian GNU/LinuxOS Version3.1
Product Version3.3 
Fixed in Version3.3.714 
Summary0000463: The garbage collection doesn't count names from the apply cache correctly.
DescriptionI ran Menaures' crasher to test 3.3-trunk and it produced some crashes, that occured only after I destroyed and restarted it. All of them were caused by a word that was arbitrarily incremented by 2. (Once a magic word was incremented by 2 resulting in a fatal error from slaballoc, or another crash was caused by a mapping width incremented by 2, so that the access to a value went somewhere beyond the mapping.)

I found out, that all of these incremented words were referenced by the apply cache and all of these entries had progp == NULL. So, as far as I could reconstruct it, the following happened:

The crasher called a function with a strange name in another object, where the function didn't existed. So, this name was entered in the apply cache with progp == NULL. After destruction of the crasher this string had only one reference left - the one from the apply cache. Now the garbage_collection was called and in count_interpreter_refs the references from the cache should be counted. But as progp==NULL note_malloced_block_ref(cache[i].name) is called first, which sets the M_REF flag of the memory block. Then count_ref_from_string(cache[i].name) should count the string reference, but as the memory block already had M_REF set and the string reference count was (still) zero, an overflow was assumed and the string reference count was not incremented. So at the end mstrings_gc_table does find this string with refcount zero and destroys it, although the cache still has a pointer to it.

After a while this memory block will be allocated for another purpose. Then, when the garbage collection is run again, counter_interpreter_refs now does count the string reference, because the assumed reference count now serves another purpose and so may have a value greater than zero (so no assumed overflow). This increments an arbitrary byte by two, which than may result in a crash.

I think it is sufficient to remove the call to note_malloced_block_ref as count_ref_from_string does the same (and more). The (very simple) patch is attached.

Greetings,
Gnomi.
TagsNo tags attached.
Attached Files
3.3gc.diff (426 bytes)   
Index: 3.3gc/src/interpret.c
===================================================================
--- 3.3gc/src/interpret.c	(Revision 2292)
+++ 3.3gc/src/interpret.c	(Arbeitskopie)
@@ -19012,8 +19012,6 @@
     int i;
 
     for (i = CACHE_SIZE; --i>= 0; ) {
-        if (!cache[i].progp)
-            note_malloced_block_ref(cache[i].name);
         if (cache[i].name)
             count_ref_from_string(cache[i].name);
     }
3.3gc.diff (426 bytes)   

Activities

lars

2006-03-18 01:22

reporter   ~0000501

The patch is exactly it.

Issue History

Date Modified Username Field Change
2006-03-16 14:15 Gnomi New Issue
2006-03-16 14:15 Gnomi File Added: 3.3gc.diff
2006-03-18 01:22 lars Status new => resolved
2006-03-18 01:22 lars Fixed in Version => 3.3.714
2006-03-18 01:22 lars Resolution open => fixed
2006-03-18 01:22 lars Assigned To => lars
2006-03-18 01:22 lars Note Added: 0000501
2007-10-06 19:55 lars Status resolved => closed
2010-11-16 09:42 lars Source_changeset_attached => ldmud.git master 76d9fb3a
2018-01-29 18:59 lars Source_changeset_attached => ldmud.git master 76d9fb3a
2018-01-29 21:57 lars Source_changeset_attached => ldmud.git master 76d9fb3a