[Forensics-changes] [yara] 97/160: Remove the "is_loaded" field from module entries to avoid multithreading issues.

Hilko Bengen bengen at moszumanska.debian.org
Sat Jul 1 10:29:22 UTC 2017


This is an automated email from the git hooks/post-receive script.

bengen pushed a commit to annotated tag v3.4.0
in repository yara.

commit ef05a686c6962dba1b721535aa06b0a5cb25068e
Author: Victor M. Alvarez <plusvic at gmail.com>
Date:   Thu Apr 30 10:29:58 2015 +0200

    Remove the "is_loaded" field from module entries to avoid multithreading issues.
    
    When two threads have the same tidx (which is fine if they are operating with different YR_RULES objects) they will collide while using the same bit within the "is_loaded" global bit mask. This may lead to one thread unloading the module loaded by the other. "is_loaded" bit mask is not actually needed, the existence of an object associated to the module in yr_modules_unload_all is enough indicator that the module was loaded.
---
 libyara/include/yara/modules.h |   2 -
 libyara/modules.c              | 107 +++++++++--------------------------------
 2 files changed, 22 insertions(+), 87 deletions(-)

diff --git a/libyara/include/yara/modules.h b/libyara/include/yara/modules.h
index 2473033..26af02a 100644
--- a/libyara/include/yara/modules.h
+++ b/libyara/include/yara/modules.h
@@ -392,8 +392,6 @@ typedef int (*YR_EXT_UNLOAD_FUNC)(
 
 typedef struct _YR_MODULE
 {
-  tidx_mask_t is_loaded;
-
   char* name;
 
   YR_EXT_DECLARATIONS_FUNC declarations;
diff --git a/libyara/modules.c b/libyara/modules.c
index ab0936d..979a6f2 100644
--- a/libyara/modules.c
+++ b/libyara/modules.c
@@ -39,7 +39,7 @@ limitations under the License.
 
 
 #define MODULE(name) \
-    { 0, \
+    { \
       #name, \
       name##__declarations, \
       name##__load, \
@@ -55,40 +55,12 @@ YR_MODULE yr_modules_table[] =
 
 #undef MODULE
 
-mutex_t yr_modules_mutex;
-
-void _yr_modules_lock()
-{
-  #ifdef _WIN32
-  WaitForSingleObject(yr_modules_mutex, INFINITE);
-  #else
-  pthread_mutex_lock(&yr_modules_mutex);
-  #endif
-}
-
-void _yr_modules_unlock()
-{
-  #ifdef _WIN32
-  ReleaseMutex(yr_modules_mutex);
-  #else
-  pthread_mutex_unlock(&yr_modules_mutex);
-  #endif
-}
-
 
 int yr_modules_initialize()
 {
-  int i, result;
-
-  #if _WIN32
-  yr_modules_mutex = CreateMutex(NULL, FALSE, NULL);
-  #else
-  pthread_mutex_init(&yr_modules_mutex, NULL);
-  #endif
-
-  for (i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
+  for (int i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
   {
-    result = yr_modules_table[i].initialize(&yr_modules_table[i]);
+    int result = yr_modules_table[i].initialize(&yr_modules_table[i]);
 
     if (result != ERROR_SUCCESS)
       return result;
@@ -100,17 +72,9 @@ int yr_modules_initialize()
 
 int yr_modules_finalize()
 {
-  int i, result;
-
-  #if _WIN32
-  CloseHandle(yr_modules_mutex);
-  #else
-  pthread_mutex_destroy(&yr_modules_mutex);
-  #endif
-
-  for (i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
+  for (int i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
   {
-    result = yr_modules_table[i].finalize(&yr_modules_table[i]);
+    int result = yr_modules_table[i].finalize(&yr_modules_table[i]);
 
     if (result != ERROR_SUCCESS)
       return result;
@@ -124,9 +88,7 @@ int yr_modules_do_declarations(
     const char* module_name,
     YR_OBJECT* main_structure)
 {
-  int i;
-
-  for (i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
+  for (int i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
   {
     if (strcmp(yr_modules_table[i].name, module_name) == 0)
       return yr_modules_table[i].declarations(main_structure);
@@ -140,13 +102,7 @@ int yr_modules_load(
     const char* module_name,
     YR_SCAN_CONTEXT* context)
 {
-  YR_MODULE_IMPORT mi;
-  YR_OBJECT* module_structure;
-
-  int result;
-  int i;
-
-  module_structure = (YR_OBJECT*) yr_hash_table_lookup(
+  YR_OBJECT* module_structure = (YR_OBJECT*) yr_hash_table_lookup(
       context->objects_table,
       module_name,
       NULL);
@@ -165,11 +121,13 @@ int yr_modules_load(
       NULL,
       &module_structure));
 
+  YR_MODULE_IMPORT mi;
+
   mi.module_name = module_name;
   mi.module_data = NULL;
   mi.module_data_size = 0;
 
-  result = context->callback(
+  int result = context->callback(
       CALLBACK_MSG_IMPORT_MODULE,
       &mi,
       context->user_data);
@@ -189,7 +147,7 @@ int yr_modules_load(
           module_structure),
       yr_object_destroy(module_structure));
 
-  for (i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
+  for (int i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
   {
     if (strcmp(yr_modules_table[i].name, module_name) == 0)
     {
@@ -201,9 +159,6 @@ int yr_modules_load(
 
       if (result != ERROR_SUCCESS)
         return result;
-      _yr_modules_lock();
-      yr_modules_table[i].is_loaded |= 1 << yr_get_tidx();
-      _yr_modules_unlock();
     }
   }
 
@@ -214,26 +169,15 @@ int yr_modules_load(
 int yr_modules_unload_all(
     YR_SCAN_CONTEXT* context)
 {
-  YR_OBJECT* module_structure;
-  tidx_mask_t tidx_mask = 1 << yr_get_tidx();
-  int i;
-
-  for (i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
+  for (int i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
   {
-    if (yr_modules_table[i].is_loaded & tidx_mask)
-    {
-      module_structure = (YR_OBJECT*) yr_hash_table_lookup(
-          context->objects_table,
-          yr_modules_table[i].name,
-          NULL);
-
-      assert(module_structure != NULL);
+    YR_OBJECT* module_structure = (YR_OBJECT*) yr_hash_table_lookup(
+        context->objects_table,
+        yr_modules_table[i].name,
+        NULL);
 
+    if (module_structure != NULL)
       yr_modules_table[i].unload(module_structure);
-      _yr_modules_lock();
-      yr_modules_table[i].is_loaded &= ~tidx_mask;
-      _yr_modules_unlock();
-    }
   }
 
   return ERROR_SUCCESS;
@@ -243,21 +187,14 @@ int yr_modules_unload_all(
 void yr_modules_print_data(
     YR_SCAN_CONTEXT* context)
 {
-  YR_OBJECT* module_structure;
-  tidx_mask_t tidx_mask = 1 << yr_get_tidx();
-
   for (int i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
   {
-    if (yr_modules_table[i].is_loaded & tidx_mask)
-    {
-      module_structure = (YR_OBJECT*) yr_hash_table_lookup(
-          context->objects_table,
-          yr_modules_table[i].name,
-          NULL);
-
-      assert(module_structure != NULL);
+    YR_OBJECT* module_structure = (YR_OBJECT*) yr_hash_table_lookup(
+        context->objects_table,
+        yr_modules_table[i].name,
+        NULL);
 
+    if (module_structure != NULL)
       yr_object_print_data(module_structure, 0);
-    }
   }
 }

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/forensics/yara.git



More information about the forensics-changes mailing list