StateClient object instances are stored in thread local storage. Each
thread can only have one instance of this object. There is however one
shared list inside of TLSOjbect2 that holds references to all
StateClient objects created in a given process. The instances of

StateClient can be destroyed in two ways.
1. Something calls dfb_gfx_cleanup().
2. Thread terminates.

In the first case the function is called once when the process using
directfb shuts down. This happens ony once during application life.
The function iterates over all StateClient objects present in the
TLSObject2::list and removes each one.

The second case libc will call the callback function passed to
direct_tls_register() when the thread which registered the tls object
terminates.

The problem is that while dfb_gfx_cleanup() does remove the deleted objects
from the TLSObject2::list, the default callback function passed to
direct_tls_register (TLSObject2::destructor) does not. As a result the
following situation will lead to a double-free memory corruption.
1. The application starts and initializes directfb.
2. At least one extra thread is started, this thread uses some dfb APIs
   which internally create new StateClient object.
3. The extra thread terminates thus invokes TLSObject2::destructor.
4. The application exits, dfb_gfx_cleanup() is called, StateClient for the
   extra thread is deleted for the second time.

The problem is nicely illustrated by valgrind. In this case dfbterm is
used as the application triggering the problem.

==336== Invalid read of size 4
==336==    at 0x4EA4120: CoreGraphicsStateClient_Deinit
                         (CoreGraphicsStateClient.cpp:387)
==336==    by 0x4F7165F: StateClient::~StateClient() (util.cpp:78)
==336==    by 0x4F71A07: Direct::TLSObject2<StateClient, StateClient,
                         StateClient>::DeleteAll() (TLSObject.h:175)
==336==    by 0x4EE3130: dfb_core_shutdown (core.c:1734)
==336==    by 0x4EE36F3: dfb_core_arena_shutdown (core.c:2020)
==336==    by 0x4EE418C: dfb_core_destroy (core.c:472)
==336==    by 0x4E969A2: IDirectFB_Destruct (idirectfb.c:307)
==336==    by 0x402813: main (term.c:523)
==336==  Address 0x8b37730 is 1,072 bytes inside a block of size 1,136 free'd
==336==    at 0x4C28D2C: operator delete(void*) (vg_replace_malloc.c:482)
==337==    by 0x5233F81: __nptl_deallocate_tsd (pthread_create.c:158)
==337==    by 0x52341E3: start_thread (pthread_create.c:325)
==336==    by 0x5B7A0DC: clone (clone.S:111)

This bug also triggers the following assertion:
Assertion [(client)->magic == D_MAGIC("CoreGraphicsStateClient")] failed
*** [CoreGraphicsStateClient.cpp:387 in
CoreGraphicsStateClient_Deinit()]

This patch is also applicable for directfb-1.7 branch.

Signed-off-by: Peter Tworek <tworaz...@gmail.com>
---
 src/gfx/util.cpp | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/src/gfx/util.cpp b/src/gfx/util.cpp
index a432dcd..5d3cc73 100644
--- a/src/gfx/util.cpp
+++ b/src/gfx/util.cpp
@@ -84,13 +84,24 @@ public:
 
      static void destroy( void *ctx, StateClient *client )
      {
-          delete client;
+          _tls.Delete();
      }
-};
 
+     static StateClient* Get()
+     {
+          return _tls.Get();
+     }
 
-static Direct::TLSObject2<StateClient> state_client_tls;
+     static void DeleteAll()
+     {
+          _tls.DeleteAll();
+     }
+
+private:
+     static Direct::TLSObject2<StateClient> _tls;
+};
 
+static Direct::TLSObject2<StateClient> StateClient::_tls;
 
 extern "C" {
 
@@ -98,13 +109,13 @@ extern "C" {
 void
 dfb_gfx_init_tls()
 {
-     state_client_tls.Get();
+     StateClient::Get();
 }
 
 void
 dfb_gfx_cleanup()
 {
-     state_client_tls.DeleteAll();
+     StateClient::DeleteAll();
 }
 
 void
@@ -140,7 +151,7 @@ dfb_gfx_copy_stereo( CoreSurface         *source,
 {
      DFBRectangle sourcerect = { 0, 0, source->config.size.w, 
source->config.size.h };
 
-     StateClient *client = state_client_tls.Get();
+     StateClient *client = StateClient::Get();
 
      D_FLAGS_SET( client->state.modified, SMF_CLIP | SMF_SOURCE | 
SMF_DESTINATION | SMF_FROM | SMF_TO );
 
@@ -180,7 +191,7 @@ dfb_gfx_clear( CoreSurface *surface, CoreSurfaceBufferRole 
role )
 {
      DFBRectangle rect = { 0, 0, surface->config.size.w, 
surface->config.size.h };
 
-     StateClient *client = state_client_tls.Get();
+     StateClient *client = StateClient::Get();
 
      D_FLAGS_SET( client->state.modified, SMF_CLIP | SMF_COLOR | 
SMF_DESTINATION | SMF_TO );
 
@@ -237,7 +248,7 @@ void dfb_gfx_stretch_stereo( CoreSurface         *source,
                return;
      }
 
-     StateClient *client = state_client_tls.Get();
+     StateClient *client = StateClient::Get();
 
      D_FLAGS_SET( client->state.modified, SMF_CLIP | SMF_SOURCE | 
SMF_DESTINATION | SMF_FROM | SMF_TO );
 
@@ -309,7 +320,7 @@ dfb_gfx_copy_regions_stereo( CoreSurface           *source,
      }
 
      if (n > 0) {
-          StateClient *client = state_client_tls.Get();
+          StateClient *client = StateClient::Get();
 
           D_FLAGS_SET( client->state.modified, SMF_CLIP | SMF_SOURCE | 
SMF_DESTINATION | SMF_FROM | SMF_TO );
 
@@ -351,7 +362,7 @@ dfb_gfx_copy_regions_client( CoreSurface             
*source,
      DFBRectangle             rect = { 0, 0, source->config.size.w, 
source->config.size.h };
      DFBRectangle             rects[num];
      DFBPoint                 points[num];
-     CoreGraphicsStateClient *client = _client ? _client : 
&state_client_tls.Get()->client;
+     CoreGraphicsStateClient *client = _client ? _client : 
&StateClient::Get()->client;
      CardState               *state  = client->state;
      CardState                backup;
 
@@ -422,7 +433,7 @@ back_to_front_copy( CoreSurface             *surface,
 {
      DFBRectangle  rect;
      DFBPoint      point;
-     StateClient  *client = state_client_tls.Get();
+     StateClient  *client = StateClient::Get();
 
 
      if (region) {
-- 
1.8.1.5

_______________________________________________
directfb-dev mailing list
directfb-dev@directfb.org
http://mail.directfb.org/cgi-bin/mailman/listinfo/directfb-dev

Reply via email to