[haiku-commits] haiku: hrev48985 - in src: kits/interface system/libroot/posix/malloc_debug kits/tracker add-ons/kernel/debugger/demangle

  • From: mmlr@xxxxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Sat, 4 Apr 2015 11:05:09 +0200 (CEST)

hrev48985 adds 6 changesets to branch 'master'
old head: 5ecdb49e5fce4d8511bfa445d8579e3287d1d08d
new head: 37acb83e4c7f8f946fda8d13f3f64cb721cc3fe3
overview:
http://cgit.haiku-os.org/haiku/log/?qt=range&q=37acb83e4c7f+%5E5ecdb49e5fce

----------------------------------------------------------------------------

23a1bcf28bc3: gcc2 demangler: Fix skip of string termination.

The inner loop to skip the function declaration stops at the
terminating null but didn't break out of the loop in that case, causing
the outer loop increment to skip the terminator and read beyond the
string end.

Well formatted symbols do not trigger this, but there sometimes are
false positives that would cause it to happen. It was seen in Debugger
that reuses this code.

46578389ba8b: BShelf: Unset the shelf of the containing view on destruction.

The BShelf sets itself as a handler on the containing view on creation
but didn't unset itself on destruction. When the containing view was
later destroyed it would unset the shelf which triggered a
use-after-free if you deleted the BShelf beforehand.

Since the ownership of a BShelf isn't explicitly documented I took the
shelf in DeskWindow of Tracker as a reference, which does delete it
explicitly.

3fc2dd56db2e: BView: Fix 64 bit build with debug output, whitespace cleanup.

d0a9f6803b66: Tracker: Fix use-after-free on destruction of the icon caches.

The hash table member still uses the element array memeber to clear
itself on destruction. We must therefore ensure that the element array
isn't destroyed before the hash table. Since the destruction order of
memebers is the reverse order of their declaration, reordering them
is enough.

2c9e11ba385e: media_addon_server: Fix use-after-free on quit.

Putting the add-ons uses the dormant node manager of the BMediaRoster.
The BMediaRoster must therefore only be quit after all add-ons are put.

37acb83e4c7f: libroot_debug: Fix build of guarded_heap after atomic changes.

[ Michael Lotz <mmlr@xxxxxxxx> ]

----------------------------------------------------------------------------

7 files changed, 23 insertions(+), 17 deletions(-)
src/add-ons/kernel/debugger/demangle/gcc2.cpp | 3 +++
src/kits/interface/Shelf.cpp | 2 ++
src/kits/interface/View.cpp | 9 +++++----
src/kits/tracker/IconCache.cpp | 12 ++++++------
src/kits/tracker/IconCache.h | 4 ++--
src/servers/media_addon/MediaAddonServer.cpp | 6 +++---
src/system/libroot/posix/malloc_debug/guarded_heap.cpp | 4 ++--

############################################################################

Commit: 23a1bcf28bc3dd1e5ded670513e4339ae6ea7540
URL: http://cgit.haiku-os.org/haiku/commit/?id=23a1bcf28bc3
Author: Michael Lotz <mmlr@xxxxxxxx>
Date: Sat Apr 4 08:34:07 2015 UTC

gcc2 demangler: Fix skip of string termination.

The inner loop to skip the function declaration stops at the
terminating null but didn't break out of the loop in that case, causing
the outer loop increment to skip the terminator and read beyond the
string end.

Well formatted symbols do not trigger this, but there sometimes are
false positives that would cause it to happen. It was seen in Debugger
that reuses this code.

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/debugger/demangle/gcc2.cpp
b/src/add-ons/kernel/debugger/demangle/gcc2.cpp
index 6b9e73b..a35a292 100644
--- a/src/add-ons/kernel/debugger/demangle/gcc2.cpp
+++ b/src/add-ons/kernel/debugger/demangle/gcc2.cpp
@@ -43,6 +43,9 @@ ignore_qualifiers(const char** _arg)
// skip function declaration
while (**_arg && **_arg != '_')
(*_arg)++;
+
+ if (**_arg == 0)
+ break;
}

(*_arg)++;

############################################################################

Commit: 46578389ba8bf906312ab1f949ffbeedc33635df
URL: http://cgit.haiku-os.org/haiku/commit/?id=46578389ba8b
Author: Michael Lotz <mmlr@xxxxxxxx>
Date: Sat Apr 4 08:40:57 2015 UTC

BShelf: Unset the shelf of the containing view on destruction.

The BShelf sets itself as a handler on the containing view on creation
but didn't unset itself on destruction. When the containing view was
later destroyed it would unset the shelf which triggered a
use-after-free if you deleted the BShelf beforehand.

Since the ownership of a BShelf isn't explicitly documented I took the
shelf in DeskWindow of Tracker as a reference, which does delete it
explicitly.

----------------------------------------------------------------------------

diff --git a/src/kits/interface/Shelf.cpp b/src/kits/interface/Shelf.cpp
index 7e51bda..99cd707 100644
--- a/src/kits/interface/Shelf.cpp
+++ b/src/kits/interface/Shelf.cpp
@@ -560,6 +560,8 @@ BShelf::~BShelf()
fReplicants.RemoveItem((int32)0);
delete data;
}
+
+ fContainerView->_SetShelf(NULL);
}



############################################################################

Commit: 3fc2dd56db2eb2df497022b7834dc9ba786e5679
URL: http://cgit.haiku-os.org/haiku/commit/?id=3fc2dd56db2e
Author: Michael Lotz <mmlr@xxxxxxxx>
Date: Sat Apr 4 08:49:38 2015 UTC

BView: Fix 64 bit build with debug output, whitespace cleanup.

----------------------------------------------------------------------------

diff --git a/src/kits/interface/View.cpp b/src/kits/interface/View.cpp
index 14ca7e2..2e60b10 100644
--- a/src/kits/interface/View.cpp
+++ b/src/kits/interface/View.cpp
@@ -4023,9 +4023,9 @@ void
BView::AddChild(BView* child, BView* before)
{
STRACE(("BView(%s)::AddChild(child '%s', before '%s')\n",
- this->Name(),
- child != NULL && child->Name() ? child->Name() : "NULL",
- before != NULL && before->Name() ? before->Name() : "NULL"));
+ this->Name(),
+ child != NULL && child->Name() ? child->Name() : "NULL",
+ before != NULL && before->Name() ? before->Name() : "NULL"));

if (!_AddChild(child, before))
return;
@@ -5783,7 +5783,8 @@ BView::_SwitchServerCurrentView() const
int32 serverToken = _get_object_token_(this);

if (fOwner->fLastViewToken != serverToken) {
- STRACE(("contacting app_server... sending token: %ld\n",
serverToken));
+ STRACE(("contacting app_server... sending token: %" B_PRId32
"\n",
+ serverToken));
fOwner->fLink->StartMessage(AS_SET_CURRENT_VIEW);
fOwner->fLink->Attach<int32>(serverToken);


############################################################################

Commit: d0a9f6803b662ad1c9b20bf5dff1d461b1d71548
URL: http://cgit.haiku-os.org/haiku/commit/?id=d0a9f6803b66
Author: Michael Lotz <mmlr@xxxxxxxx>
Date: Sat Apr 4 08:51:24 2015 UTC

Tracker: Fix use-after-free on destruction of the icon caches.

The hash table member still uses the element array memeber to clear
itself on destruction. We must therefore ensure that the element array
isn't destroyed before the hash table. Since the destruction order of
memebers is the reverse order of their declaration, reordering them
is enough.

----------------------------------------------------------------------------

diff --git a/src/kits/tracker/IconCache.cpp b/src/kits/tracker/IconCache.cpp
index 0ce95ce..e09980f 100644
--- a/src/kits/tracker/IconCache.cpp
+++ b/src/kits/tracker/IconCache.cpp
@@ -1408,13 +1408,13 @@ SharedIconCache::SharedIconCache()
:
#if DEBUG
SimpleIconCache("Shared icon cache aka \"The Dead-Locker\""),
- fHashTable(20),
fElementArray(20),
+ fHashTable(20),
fRetiredBitmaps(20, true)
#else
SimpleIconCache("Tracker shared icon cache"),
- fHashTable(1000),
fElementArray(1024),
+ fHashTable(1000),
fRetiredBitmaps(256, true)
#endif
{
@@ -1765,12 +1765,12 @@ NodeIconCache::NodeIconCache()
:
#if DEBUG
SimpleIconCache("Node icon cache aka \"The Dead-Locker\""),
- fHashTable(20),
- fElementArray(20)
+ fElementArray(20),
+ fHashTable(20)
#else
SimpleIconCache("Tracker node icon cache"),
- fHashTable(100),
- fElementArray(100)
+ fElementArray(100),
+ fHashTable(100)
#endif
{
fHashTable.SetElementVector(&fElementArray);
diff --git a/src/kits/tracker/IconCache.h b/src/kits/tracker/IconCache.h
index d803ba7..e14f3fb 100644
--- a/src/kits/tracker/IconCache.h
+++ b/src/kits/tracker/IconCache.h
@@ -262,8 +262,8 @@ public:
void RemoveAliasesTo(int32 index);

private:
- OpenHashTable<SharedCacheEntry, SharedCacheEntryArray> fHashTable;
SharedCacheEntryArray fElementArray;
+ OpenHashTable<SharedCacheEntry, SharedCacheEntryArray> fHashTable;
BObjectList<BBitmap> fRetiredBitmaps;
// icons are drawn asynchronously, can't just delete them right
away,
// instead have to place them onto the retired bitmap list and
wait
@@ -336,8 +336,8 @@ public:
void RemoveAliasesTo(int32 index);

private:
- OpenHashTable<NodeCacheEntry, NodeCacheEntryArray> fHashTable;
NodeCacheEntryArray fElementArray;
+ OpenHashTable<NodeCacheEntry, NodeCacheEntryArray> fHashTable;
};



############################################################################

Commit: 2c9e11ba385eae2dd646d7c5c4a711ffd0c655ae
URL: http://cgit.haiku-os.org/haiku/commit/?id=2c9e11ba385e
Author: Michael Lotz <mmlr@xxxxxxxx>
Date: Sat Apr 4 08:57:50 2015 UTC

media_addon_server: Fix use-after-free on quit.

Putting the add-ons uses the dormant node manager of the BMediaRoster.
The BMediaRoster must therefore only be quit after all add-ons are put.

----------------------------------------------------------------------------

diff --git a/src/servers/media_addon/MediaAddonServer.cpp
b/src/servers/media_addon/MediaAddonServer.cpp
index 9924bb5..475add7 100644
--- a/src/servers/media_addon/MediaAddonServer.cpp
+++ b/src/servers/media_addon/MediaAddonServer.cpp
@@ -335,12 +335,12 @@ MediaAddonServer::QuitRequested()
fSystemTimeSource = NULL;
}

- BMediaRoster::CurrentRoster()->Lock();
- BMediaRoster::CurrentRoster()->Quit();
-
for (iterator = fInfoMap.begin(); iterator != fInfoMap.end();
iterator++)
_PutAddonIfPossible(iterator->second);

+ BMediaRoster::CurrentRoster()->Lock();
+ BMediaRoster::CurrentRoster()->Quit();
+
return true;
}


############################################################################

Revision: hrev48985
Commit: 37acb83e4c7f8f946fda8d13f3f64cb721cc3fe3
URL: http://cgit.haiku-os.org/haiku/commit/?id=37acb83e4c7f
Author: Michael Lotz <mmlr@xxxxxxxx>
Date: Sat Apr 4 09:00:40 2015 UTC

libroot_debug: Fix build of guarded_heap after atomic changes.

----------------------------------------------------------------------------

diff --git a/src/system/libroot/posix/malloc_debug/guarded_heap.cpp
b/src/system/libroot/posix/malloc_debug/guarded_heap.cpp
index f0727cb..2d5600a 100644
--- a/src/system/libroot/posix/malloc_debug/guarded_heap.cpp
+++ b/src/system/libroot/posix/malloc_debug/guarded_heap.cpp
@@ -252,7 +252,7 @@ guarded_heap_free_page(guarded_heap_area& area, size_t
pageIndex,
static void
guarded_heap_pages_allocated(guarded_heap& heap, size_t pagesAllocated)
{
- atomic_add((vint32*)&heap.used_pages, pagesAllocated);
+ atomic_add((int32*)&heap.used_pages, pagesAllocated);
}


@@ -529,7 +529,7 @@ guarded_heap_area_free(guarded_heap_area& area, void*
address)

if (area.heap->reuse_memory) {
area.used_pages -= pagesFreed;
- atomic_add((vint32*)&area.heap->used_pages, -pagesFreed);
+ atomic_add((int32*)&area.heap->used_pages, -pagesFreed);
}

return true;


Other related posts: