The 'fix' for sharedmem
David Hakim <dhakim-Gkm/TONP9n1Wk0Htik3J/[email protected]> Tue, 4 Feb 2003 17:02:41 -0500
| Newsgroups | gmane.comp.lang.moto.devel |
|---|---|
| Message-ID | <[email protected]> |
Ok ... so I've been thinking more about bookkeeping in shared memory
and that pathological case I mentioned just seems more and more
pathological to me. Here's why:
Fact : Whenever someone free()s memory that either
1) Has already been de-allocated
2) Or was never allocated in the first place
Bad things CAN happen. This will be true in any memory management
system that allows for explicit free. The reason is ... sometimes the
free might work! This would be the case when some other part of the
program allocated some memory and got back just the address now
attempting to be freed. This sort of bug in client code ALMOST ALWAYS
causes problems somewhere.
The question is "Can we guarantee the integrity of the memory manager
even when bad client code is written against it" ? The answer to this
is yes ... but we might not want to.
First off here's why its yes. We can always keep a list somewhere
(outside of the area of shared memory where objects are allocated) of
every piece of memory currently allocated and every time we get a
'free()' call we check the list to make sure the address being free()d
is in there.
It turns out the above algorithm would be fairly complex and expensive
to implement. Incurring significant (although constant) extra costs on
every malloc() and free().
Shared memory already maintains efficient allocated and free lists. The
free list is implemented as a splay tree organized on free node sizes.
It is located on the high end of the shared segment. This is outside of
the space where memory allocated with malloc() goes.
The allocated list on the other hand is interspersed with memory
allocated with malloc(). In fact the nodes for the alist are explicitly
allocated immediately prior to the object itself. This was designed
precisely for performance so that the bookkeeping for allocated
addresses could be found in constant time when an address was freed.
You can get to the anode for an object by simple pointer arithmetic on
the object's address. This has always meant that a evil extension
author could corrupt the bookkeeping by doing the same pointer
arithmetic on addresses he got back from malloc().
Anyhow, an ANode is nothing more than a node in a doubly linked list.
typedef struct anode {
struct anode* prev;
struct anode* next;
size_t size;
void* pool;
void(*freefn)(void *);
} ANode;
When memory gets free()'d we want to remove the ANode associated with
the memory being freed. Right now there is an implicit assumption in
the code that the memory being free()'d was allocated. So shared_free
goes right into the process of re-linking the previous and next nodes
in the list:
startAddress = anode = address - sizeof(ANode);
endAddress = address + anode->size;
/* Delete the anode */
if(anode->prev != NULL){
anode->prev->next = anode->next;
lowerAddress = (void*)anode->prev + anode->prev->size +
sizeof(ANode);
} else {
*ALIST_HEAD = anode->next;
lowerAddress = BASE_ADDRESS;
}
if(anode->next != NULL) {
anode->next->prev = anode->prev;
higherAddress = anode->next;
} else {
*ALIST_TAIL = anode->prev;
higherAddress = BASE_ADDRESS+SHARED_SEGMENT;
}
...
But what if the address was never allocated ? Or one that had already
been freed ? This is when bad things happen currently. The question is
how do we tell if that's the case ?
We could tell 99.9999% of the time by checking the following :
1) Is anode->prev not null and inside of shared memory - if no than
this anode is invalid
1.1) if anode->prev is null than this should be the first allocated
node in shared memory, thus ALIST_HEAD should point to it. If it
doesn't than this node has already been freed (or was never allocated
to begin with)
1.2) if anode->prev is not null than anode->prev->next should == anode.
If this is not the case than this node cannot be part of the AList
(meaning it had already been freed or was never allocated to begin with)
Equivalent checks can be done on the subsequent node as well. Here is
the C code needed to do all the checks :
int isAddressAllocated(void* address){
int validAddress = 1;
ANode* anode = address - sizeof(ANode);
if(shared_check(anode)){
if(anode->prev != NULL) {
if(!shared_check(anode->prev)) validAddress = 0;
else if(anode->prev->next != anode) validAddress = 0;
} else if (*ALIST_HEAD != anode) validAddress = 0;
if(anode->next != NULL) {
if(!shared_check(anode->next)) validAddress = 0;
else if(anode->next->prev != anode) validAddress = 0;
} else if (*ALIST_TAIL != anode) validAddress = 0;
} else validAddress = 0;
return validAddress;
}
So when do these checks not work ?
These checks will fail when someone, somehow re-creates a mini ANode
list by hand with three nodes in it and tries to free the middle one.
An evil extension programmer could do this for instance ... but then
again an evil extension coder could simply wipe out the entire shared
memory segment if he wanted to.
So my logic in going with the 99.99% fix is as follows. Extension
programmers could really never exploit this by accident. They would
have to want to cause memory manager corruption. And even if by some
wierd random chance a 3 node AList magically appeared out of thin air
and they accidentally free'd the middle node, its STILL THEIR FAULT
because they are attempting to free something they never allocated (or
already freed). So with these fixes it now takes an AMAZING COINCIDENCE
+ AN EXTENSION CODE BUG to corrupt the memory manager (as opposed to
before when it just took an extension code bug)
The above tests cause no visible performance degradation and were easy
to implement so I'm going with those for now. If anyone wants to try
the 100% fix and can show it causes no performance degradation ... I
would be the first to swap out my fix for it :)
Anyone who modifies sharedmem.c should always run the objinst
performance test (at least) afterwards. I've attached the test times
both pre and post this fix.
-Dave
Here is the timing information for objinst, a big object allocation
test, when no protection for double free's are implemented in
sharedmem.c
% ./compile.sh objinst
objinst.c:95: warning: ANSI C forbids newline in string constant
objinst.moto:98:1: warning: multi-line string literals are deprecated
timing objinst
motoc : 0.590u 0.110s 0:01.06 66.0% 0+0k 0+3io 0pf+0w
perl : 7.180u 0.020s 0:07.96 90.4% 0+0k 5+0io 0pf+0w
java : 0.390u 0.090s 0:00.67 71.6% 0+0k 2+0io 0pf+0w
python : 3.170u 0.080s 0:04.67 69.5% 0+0k 43+2io 0pf+0w
ruby : 3.900u 0.060s 0:04.57 86.6% 0+0k 0+0io 0pf+0w
php : 4.750u 0.030s 0:05.55 86.1% 0+0k 1+0io 0pf+0w
[ David-Hakims-Computer: /Users/dhakim/Workarea/perftests ]
% ./compile.sh objinst
objinst.c:95: warning: ANSI C forbids newline in string constant
objinst.moto:98:1: warning: multi-line string literals are deprecated
timing objinst
motoc : 0.630u 0.080s 0:00.85 83.5% 0+0k 0+1io 0pf+0w
perl : 7.120u 0.040s 0:07.82 91.5% 0+0k 0+0io 0pf+0w
java : 0.370u 0.100s 0:00.61 77.0% 0+0k 0+0io 0pf+0w
python : 3.170u 0.040s 0:03.47 92.5% 0+0k 0+0io 0pf+0w
ruby : 3.790u 0.050s 0:06.26 61.3% 0+0k 0+0io 0pf+0w
php : 4.670u 0.030s 0:05.25 89.5% 0+0k 0+1io 0pf+0w
[ David-Hakims-Computer: /Users/dhakim/Workarea/perftests ]
% ./compile.sh objinst
objinst.c:95: warning: ANSI C forbids newline in string constant
objinst.moto:98:1: warning: multi-line string literals are deprecated
timing objinst
motoc : 0.630u 0.090s 0:00.91 79.1% 0+0k 0+1io 0pf+0w
perl : 7.210u 0.040s 0:07.78 93.1% 0+0k 0+0io 0pf+0w
java : 0.320u 0.120s 0:00.67 65.6% 0+0k 0+0io 0pf+0w
python : 3.190u 0.020s 0:03.68 87.2% 0+0k 0+0io 0pf+0w
ruby : 3.970u 0.050s 0:04.49 89.5% 0+0k 0+0io 0pf+0w
php : 4.790u 0.020s 0:05.59 86.0% 0+0k 0+0io 0pf+0w
Post 99% fix:
objinst.c:95: warning: ANSI C forbids newline in string constant
objinst.moto:98:1: warning: multi-line string literals are deprecated
timing objinst
motoc : 0.660u 0.110s 0:00.86 89.5% 0+0k 0+1io 0pf+0w
perl : 7.200u 0.030s 0:07.93 91.1% 0+0k 0+0io 0pf+0w
java : 0.380u 0.100s 0:00.62 77.4% 0+0k 1+0io 0pf+0w
python : 3.190u 0.050s 0:03.52 92.0% 0+0k 0+2io 0pf+0w
ruby : 3.960u 0.030s 0:04.65 85.8% 0+0k 0+0io 0pf+0w
php : 4.770u 0.010s 0:05.44 87.8% 0+0k 1+0io 0pf+0w
[ David-Hakims-Computer: /Users/dhakim/Workarea/perftests ]
% ./compile.sh objinst
objinst.c:95: warning: ANSI C forbids newline in string constant
objinst.moto:98:1: warning: multi-line string literals are deprecated
timing objinst
motoc : 0.680u 0.080s 0:00.90 84.4% 0+0k 0+1io 0pf+0w
perl : 7.220u 0.000s 0:07.95 90.8% 0+0k 0+0io 0pf+0w
java : 0.330u 0.130s 0:00.70 65.7% 0+0k 0+0io 0pf+0w
python : 3.210u 0.040s 0:03.50 92.8% 0+0k 0+0io 0pf+0w
ruby : 4.040u 0.030s 0:04.70 86.5% 0+0k 0+0io 0pf+0w
php : 4.780u 0.030s 0:05.44 88.4% 0+0k 0+0io 0pf+0w
[ David-Hakims-Computer: /Users/dhakim/Workarea/perftests ]
% ./compile.sh objinst
objinst.c:95: warning: ANSI C forbids newline in string constant
objinst.moto:98:1: warning: multi-line string literals are deprecated
timing objinst
motoc : 0.670u 0.090s 0:00.90 84.4% 0+0k 0+1io 0pf+0w
perl : 7.120u 0.010s 0:08.12 87.8% 0+0k 0+0io 0pf+0w
java : 0.320u 0.140s 0:00.69 66.6% 0+0k 0+0io 0pf+0w
python : 3.160u 0.040s 0:03.80 84.2% 0+0k 0+2io 0pf+0w
ruby : 4.070u 0.050s 0:04.70 87.6% 0+0k 0+2io 0pf+0w
php : 4.640u 0.020s 0:05.76 80.9% 0+0k 0+0io 0pf+0w