#6 fix for another segfault in DropOldData

closed
nobody
None
5
2004-01-08
2004-01-02
No

DropOldData can be made to dereference a free()d
pointer, resulting in a segfault. (note this is a
different bug to the one fixed by patch number 834192)

Bug details:

The bug occurs if the block beginning at line 330 of
bandwidthd.c (in release 1.1.7) unlinks and frees the
second node in the IPDataStore linked list. Unlinking
the second node causes the while loop at line 328 to
process the first node in the list again (due to the
"rewind" at line 337).

The trouble starts with the test on line 330, which
detects whether we're processing the first node by
checking if PrevDataStore is null. In this particular
scenario, PrevDataStore is non-null, even though we are
processing the first node.

The code will then attempt to unlink the first node,
which results in DataStore pointing at free()d memory,
causing a segfault.

Fix:

One fix is to insert a 'break' at line 338 to avoid the
unnecessary trip around the loop:

--- bandwidthd-1.1.7/bandwidthd.c Tue Dec 2
16:46:12 2003
+++ bandwidthd-1.1.7-hr/bandwidthd.c Fri Jan 2
18:10:33 2004
@@ -334,7 +334,7 @@

free(DataStore->FirstBlock->Data); // Free the memory

free(DataStore->FirstBlock);
free(DataStore);
DataStore =
PrevDataStore; // Rewind
back to the prev data store so the move next below
doesn't skip a node
+ break; // move on to
the next block
}
else if
(!DataStore->FirstBlock->Next)
{

Happy new year, all.

/Hannes

Discussion

  • Hannes Reich

    Hannes Reich - 2004-01-02

    Logged In: YES
    user_id=844136

    Attaching the patch as a file, as it got line-mangled.

     
  • David R Hinkle

    David R Hinkle - 2004-01-08

    Logged In: YES
    user_id=863707

    Thanks much for pointing this out Hannes, I bet this was
    stinging alot of people. I've decided that since it's only a
    single linked list, hacking it up to allow us to traverse
    backwards is just a really bad idea. After spending some
    time looking over the code, I've realised that the rewind is not
    neccisary at all. The fact that the code stays in the internal
    while loop will make sure the next element gets evaluated.
    So, I've put a fix for this in cvs that basicly removes the
    rewind and the bugs that come with it.

    Thanks again
    David

     
  • David R Hinkle

    David R Hinkle - 2004-01-08
    • status: open --> pending
     
  • David R Hinkle

    David R Hinkle - 2004-01-08
    • status: pending --> closed
     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks