Changeset 228 for trunk

Timestamp:

04/18/11 16:25:46 (14 years ago)

Author:

tim

Message:

added a test case for pyregfi multithreaded use
updated the regfi multithreaded test case
fixed several ctypes interface problems in pyregfi
added locking to pyregfi iterators for thread safety
fixed regfi talloc race conditions with an additional lock

Location:

trunk

Files:

• include/regfi.h (modified) (5 diffs)
• lib/regfi.c (modified) (11 diffs)
• python/pyregfi/__init__.py (modified) (22 diffs)
• python/pyregfi/structures.py (modified) (4 diffs)
• src/reglookup-recover.c (modified) (3 diffs)
• src/reglookup.c (modified) (10 diffs)

trunk/include/regfi.h

@@ -821 +821 @@
   pthread_mutex_t sk_lock;
 
+  /* Needed to protect various talloc calls */
+  pthread_mutex_t mem_lock;
+
 } REGFI_FILE;
 
@@ -971 +974 @@
 /** Frees a record previously returned by one of the API functions.
  *
- * Can be used to free REGFI_NK, REGFI_VK, REGFI_SK, REGFI_DATA, and
- * REGFI_CLASSNAME records.
- *
- * @note The "const" in the data type is a bit misleading and is there just for
+ * @param file The file from which the record originated.  
+ *             (This is needed for memory management reasons.)
+ * 
+ * @param record Any of the following record types: REGFI_NK, REGFI_VK, 
+ *        REGFI_SK, REGFI_DATA, and REGFI_CLASSNAME records.
+ *
+ * @note The "const" in the record data type is a bit misleading and is there just for
  * convenience.  Since records returned previously must not be modified by users
  * of the API due to internal caching, these are returned as const, so this
@@ -982 +988 @@
  */
 _EXPORT
-void regfi_free_record(const void* record);
+void regfi_free_record(REGFI_FILE* file, const void* record);
 
 
@@ -992 +998 @@
  * without requiring them to be in sync with when it is freed.
  *
- * Can be used on REGFI_NK, REGFI_VK, REGFI_SK, REGFI_DATA, and
- * REGFI_CLASSNAME records.
+ * @param file The file from which the record originated.  
+ *             (This is needed for memory management reasons.)
+ * 
+ * @param record Any of the following record types: REGFI_NK, REGFI_VK, 
+ *        REGFI_SK, REGFI_DATA, and REGFI_CLASSNAME records.
  *
  * @return true on success, false otherwise
@@ -1000 +1009 @@
  */
 _EXPORT
-bool regfi_reference_record(const void* record);
+bool regfi_reference_record(REGFI_FILE* file, const void* record);
 
 

trunk/lib/regfi.c

@@ -1617 +1617 @@
   }
 
+  if(pthread_mutex_init(&rb->mem_lock, NULL) != 0)
+  {
+    regfi_log_add(REGFI_LOG_ERROR, "Failed to create mem_lock mutex.");
+    goto fail;
+  }
+
   rb->hbins = range_list_new();
   if(rb->hbins == NULL)
@@ -1657 +1663 @@
   pthread_rwlock_destroy(&rb->hbins_lock);
   pthread_mutex_destroy(&rb->sk_lock);
+  pthread_mutex_destroy(&rb->mem_lock);
 
   range_list_free(rb->hbins);
@@ -1721 +1728 @@
 /******************************************************************************
  *****************************************************************************/
-void regfi_free_record(const void* record)
-{
+void regfi_free_record(REGFI_FILE* file, const void* record)
+{
+  if(!regfi_lock(file, &file->mem_lock, "regfi_free_record"))
+    return;
+
   talloc_unlink(NULL, (void*)record);
+
+  regfi_unlock(file, &file->mem_lock, "regfi_free_record");
 }
 
@@ -1729 +1741 @@
 /******************************************************************************
  *****************************************************************************/
-bool regfi_reference_record(const void* record)
-{
+bool regfi_reference_record(REGFI_FILE* file, const void* record)
+{
+  bool ret_val = false;
+  if(!regfi_lock(file, &file->mem_lock, "regfi_reference_record"))
+    return ret_val;
+  
   if(talloc_reference(NULL, record) != NULL)
-    return true;
-  return false;
+    ret_val = true;
+
+  regfi_unlock(file, &file->mem_lock, "regfi_reference_record");
+  return ret_val;
 }
 
@@ -1823 +1841 @@
 void regfi_iterator_free(REGFI_ITERATOR* i)
 {
-  talloc_free(i);
+  talloc_unlink(NULL, i);
 }
 
@@ -1854 +1872 @@
     talloc_unlink(NULL, subkey);
     return false;
-  }
+  }  
   talloc_reparent(NULL, i, subkey);
 
@@ -1875 +1893 @@
     return false;
 
+  if(!regfi_lock(i->f, &i->f->mem_lock, "regfi_iterator_up"))
+    return false;
+  
   talloc_unlink(i, i->cur_key);
+  regfi_unlock(i->f, &i->f->mem_lock, "regfi_iterator_up");
+
   i->cur_key = pos->nk;
   i->cur_subkey = pos->cur_subkey;
@@ -1943 +1966 @@
 const REGFI_NK* regfi_iterator_cur_key(REGFI_ITERATOR* i)
 {
-  return talloc_reference(NULL, i->cur_key);
+  const REGFI_NK* ret_val = NULL;
+  if(!regfi_lock(i->f, &i->f->mem_lock, "regfi_iterator_cur_key"))
+    return ret_val;
+
+  ret_val = talloc_reference(NULL, i->cur_key);
+
+  regfi_unlock(i->f, &i->f->mem_lock, "regfi_iterator_cur_key");  
+  return ret_val;
 }
 
@@ -2174 +2204 @@
     }
 
-    regfi_free_record(cur);
+    regfi_free_record(file, cur);
   }
 
@@ -2211 +2241 @@
     }
 
-    regfi_free_record(cur);
+    regfi_free_record(file, cur);
   }
 
@@ -2257 +2287 @@
 {
   if(key != NULL && key->parent_off != REGFI_OFFSET_NONE)
-  {
-    /*    fprintf(stderr, "key->parent_off=%.8X\n", key->parent_off);*/
     return regfi_load_key(file, 
                           key->parent_off+REGFI_REGF_SIZE,
                           file->string_encoding, true);
-  }
-  
+
   return NULL;
 }

trunk/python/pyregfi/__init__.py

@@ -95 +95 @@
 import sys
 import time
-from pyregfi.structures import *
-
 import ctypes
 import ctypes.util
+import threading
+from pyregfi.structures import *
+
 
 ## An enumeration of registry Value data types
@@ -218 +219 @@
 
 
-
 ## Abstract class for most objects returned by the library
 class _StructureWrapper(object):
@@ -239 +239 @@
     # Memory management for most regfi structures is taken care of here
     def __del__(self):
-        regfi.regfi_free_record(self._base)
+        regfi.regfi_free_record(self._hive.file, self._base)
 
 
@@ -299 +299 @@
                             + "Current log:\n" + GetLogMessages())
         
-        if not regfi.regfi_reference_record(key._base):
+        if not regfi.regfi_reference_record(key._hive.file, key._base):
             raise Exception("Could not create _GenericList; memory error."
                             + "Current log:\n" + GetLogMessages())
@@ -308 +308 @@
     
     def __del__(self):
-        regfi.regfi_free_record(self._key_base)
-    
+        regfi.regfi_free_record(self._hive.file, self._key_base)
+
 
     ## Length of list
@@ -486 +486 @@
                 ret_val = _buffer2bytearray(cn_struct.raw,
                                             cn_struct.size)
-            regfi.regfi_free_record(cn_p)
+            regfi.regfi_free_record(self._hive.file, cn_p)
 
         return ret_val
@@ -581 +581 @@
                                         data_struct.interpreted_size)
 
-        regfi.regfi_free_record(data_p)
+        regfi.regfi_free_record(self._hive.file, data_p)
         return ret_val
     
@@ -599 +599 @@
         ret_val = _buffer2bytearray(data_struct.raw,
                                     data_struct.size)
-        regfi.regfi_free_record(data_p)
+        regfi.regfi_free_record(self._hive.file, data_p)
         return ret_val
 
@@ -747 +747 @@
     _iter = None
     _iteration_root = None
+    _lock = None
 
     def __init__(self, hive):
-        self._iter = regfi.regfi_iterator_new(hive.file, REGFI_ENCODING_UTF8)
+        self._iter = regfi.regfi_iterator_new(hive.file)
         if not self._iter:
             raise Exception("Could not create iterator.  Current log:\n"
                             + GetLogMessages())
         self._hive = hive
-        
+        self._lock = threading.RLock()
+    
     def __getattr__(self, name):
-        return getattr(self.file.contents, name)
-
-    def __del__(self):    
+        self._lock.acquire()
+        ret_val = getattr(self._iter.contents, name)
+        self._lock.release()
+        return ret_val
+
+    def __del__(self):
+        self._lock.acquire()
         regfi.regfi_iterator_free(self._iter)
+        self._lock.release()
 
     def __iter__(self):
+        self._lock.acquire()
         self._iteration_root = None
+        self._lock.release()
         return self
 
     def __next__(self):
+        self._lock.acquire()
         if self._iteration_root == None:
-            self._iteration_root = self.current_key()
+            self._iteration_root = self.current_key().offset
         elif not regfi.regfi_iterator_down(self._iter):
             up_ret = regfi.regfi_iterator_up(self._iter)
             while (up_ret and
                    not regfi.regfi_iterator_next_subkey(self._iter)):
-                if self._iteration_root == self.current_key():
+                if self._iteration_root == self.current_key().offset:
                     self._iteration_root = None
+                    self._lock.release()
                     raise StopIteration('')
                 up_ret = regfi.regfi_iterator_up(self._iter)
@@ -779 +790 @@
             if not up_ret:
                 self._iteration_root = None
+                self._lock.release()
                 raise StopIteration('')
             
             # XXX: Use non-generic exception
             if not regfi.regfi_iterator_down(self._iter):
+                self._lock.release()
                 raise Exception('Error traversing iterator downward.'+
                                 ' Current log:\n'+ GetLogMessages())
 
         regfi.regfi_iterator_first_subkey(self._iter)
-        return self.current_key()
+        ret_val = self.current_key()
+        self._lock.release()
+
+        return ret_val
+
 
     # For Python 2.x
@@ -806 +823 @@
     # @return True if successful, False otherwise
     def down(self, subkey_name=None):
+        ret_val = None
         if subkey_name == None:
-            return regfi.regfi_iterator_down(self._iter)
+            self._lock.acquire()
+            ret_val = regfi.regfi_iterator_down(self._iter)
         else:
             if name != None:
                 name = name.encode('utf-8')
-            return (regfi.regfi_iterator_find_subkey(self._iter, name) 
-                    and regfi.regfi_iterator_down(self._iter))
+            self._lock.acquire()
+            ret_val = (regfi.regfi_iterator_find_subkey(self._iter, name) 
+                       and regfi.regfi_iterator_down(self._iter))
+        
+        self._lock.release()
+        return ret_val
 
 
@@ -824 +847 @@
     #       default selections.
     def up(self):
-        return regfi.regfi_iterator_up(self._iter)
+        self._lock.acquire()
+        ret_val = regfi.regfi_iterator_up(self._iter)
+        self._lock.release()
+        return ret_val
 
 
@@ -832 +858 @@
     #         None on error or if the current key has no subkeys.
     def first_subkey(self):
+        ret_val = None
+        self._lock.acquire()
         if regfi.regfi_iterator_first_subkey(self._iter):
-            return self.current_subkey()
-        return None
+            ret_val = self.current_subkey()
+        self._lock.release()
+        return ret_val
 
 
@@ -842 +871 @@
     #         None on error or if the current key has no values.
     def first_value(self):
+        ret_val = None
+        self._lock.acquire()
         if regfi.regfi_iterator_first_value(self._iter):
-            return self.current_value()
-        return None
+            ret_val = self.current_value()
+        self._lock.release()
+        return ret_val
 
 
@@ -852 +884 @@
     #         None if there are no remaining subkeys or an error occurred.
     def next_subkey(self):
+        ret_val = None
+        self._lock.acquire()
         if regfi.regfi_iterator_next_subkey(self._iter):
-            return self.current_subkey()
-        return None
+            ret_val = self.current_subkey()
+        self._lock.release()
+        return ret_val
 
 
@@ -862 +897 @@
     #         None if there are no remaining values or an error occurred.
     def next_value(self):
+        ret_val = None
+        self._lock.acquire()
         if regfi.regfi_iterator_next_value(self._iter):
-            return self.current_value()
-        return None
+            ret_val = self.current_value()
+        self._lock.release()
+        return ret_val
 
 
@@ -874 +912 @@
         if name != None:
             name = name.encode('utf-8')
+        ret_val = None
+        self._lock.acquire()
         if regfi.regfi_iterator_find_subkey(self._iter, name):
-            return self.current_subkey()
-        return None
+            ret_val = self.current_subkey()
+        self._lock.release()
+        return ret_val
 
 
@@ -886 +927 @@
         if name != None:
             name = name.encode('utf-8')
+        ret_val = None
+        self._lock.acquire()
         if regfi.regfi_iterator_find_value(self._iter, name):
-            return self.current_value()
-        return None
+            ret_val = self.current_value()
+        self._lock.release()
+        return ret_val
 
     ## Retrieves the currently selected subkey
@@ -894 +938 @@
     # @return A Key instance of the current subkey
     def current_subkey(self):
-        return Key(self._hive, regfi.regfi_iterator_cur_subkey(self._iter))
+        self._lock.acquire()
+        ret_val = Key(self._hive, regfi.regfi_iterator_cur_subkey(self._iter))
+        self._lock.release()
+        return ret_val
 
     ## Retrieves the currently selected value
@@ -900 +947 @@
     # @return A Value instance of the current value
     def current_value(self):
-        return Value(self._hive, regfi.regfi_iterator_cur_value(self._iter))
+        self._lock.acquire()
+        ret_val = Value(self._hive, regfi.regfi_iterator_cur_value(self._iter))
+        self._lock.release()
+        return ret_val
 
     ## Retrieves the current key
@@ -906 +956 @@
     # @return A Key instance of the current position of the iterator
     def current_key(self):
-        return Key(self._hive, regfi.regfi_iterator_cur_key(self._iter))
+        self._lock.acquire()
+        ret_val = Key(self._hive, regfi.regfi_iterator_cur_key(self._iter))
+        self._lock.release()
+        return ret_val
 
 
@@ -919 +972 @@
         cpath = _strlist2charss(path)
 
-        # XXX: Use non-generic exception
-        if not regfi.regfi_iterator_walk_path(self._iter, cpath):
+        self._lock.acquire()
+        result = regfi.regfi_iterator_walk_path(self._iter, cpath)
+        self._lock.release()
+        if not result:
+            # XXX: Use non-generic exception
             raise Exception('Could not locate path.\n'+GetLogMessages())
 

trunk/python/pyregfi/structures.py

@@ -171 +171 @@
 REGFI_VALUE_LIST._fields_ = [('offset', c_uint32),
                              ('cell_size', c_uint32),
-                             ('num_children', c_uint32),
                              ('num_values', c_uint32),
                              ('elements', c_void_p),
@@ -247 +246 @@
 regfi.regfi_get_rootkey.restype = POINTER(REGFI_NK)
 
-regfi.regfi_free_record.argtypes = [c_void_p]
+regfi.regfi_free_record.argtypes = [POINTER(REGFI_FILE), c_void_p]
 regfi.regfi_free_record.restype = None
 
-regfi.regfi_reference_record.argtypes = [c_void_p]
+regfi.regfi_reference_record.argtypes = [POINTER(REGFI_FILE), c_void_p]
 regfi.regfi_reference_record.restype = c_bool
 
@@ -290 +289 @@
 regfi.regfi_nt2unix_time.restype = c_double
 
-regfi.regfi_iterator_new.argtypes = [POINTER(REGFI_FILE), REGFI_ENCODING]
+regfi.regfi_iterator_new.argtypes = [POINTER(REGFI_FILE)]
 regfi.regfi_iterator_new.restype = POINTER(REGFI_ITERATOR)
 
@@ -305 +304 @@
 regfi.regfi_iterator_to_root.restype = c_bool
 
-regfi.regfi_iterator_walk_path.argtypes = [POINTER(REGFI_ITERATOR)]
+regfi.regfi_iterator_walk_path.argtypes = [POINTER(REGFI_ITERATOR), POINTER(c_char_p)]
 regfi.regfi_iterator_walk_path.restype = c_bool
 

trunk/src/reglookup-recover.c

@@ -291 +291 @@
         {
           /* XXX: Need to add a warning here */
-          regfi_free_record(cur_ancestor);
+          regfi_free_record(f, cur_ancestor);
           void_stack_free(path_stack);
           return NULL;
@@ -303 +303 @@
         ret_val_size += path_element->len + 1;
 
-        regfi_free_record(cur_ancestor);
+        regfi_free_record(f, cur_ancestor);
       }
     }
@@ -631 +631 @@
 
  fail:
-  regfi_free_record(key);
+  regfi_free_record(f, key);
   return error_code;
 }

trunk/src/reglookup.c

@@ -95 +95 @@
       fprintf(stderr, "WARN: While quoting value for '%s/%s', "
               "warning returned: %s\n", prefix, quoted_name, conv_error);
-    regfi_free_record(data);
+    regfi_free_record(iter->f, data);
   }
 
@@ -282 +282 @@
     if(!type_filter_enabled || (value->type == type_filter))
       printValue(iter, value, prefix);
-    regfi_free_record(value);
+    regfi_free_record(iter->f, value);
     regfi_iterator_next_value(iter);
     printMsgs(iter->f);
@@ -310 +310 @@
     sacl = regfi_get_sacl(sk->sec_desc);
     dacl = regfi_get_dacl(sk->sec_desc);
-    regfi_free_record(sk);
+    regfi_free_record(iter->f, sk);
 
     if(owner == NULL)
@@ -345 +345 @@
     else
       quoted_classname = empty_str;
-    regfi_free_record(classname);
+    regfi_free_record(iter->f, classname);
 
     printMsgs(iter->f);
@@ -365 +365 @@
     printf("%s,KEY,,%s\n", full_path, mtime);
 
-  regfi_free_record(key);
+  regfi_free_record(iter->f, key);
 }
 
@@ -406 +406 @@
       if(cur != root)
       {
-        regfi_free_record(cur);
+        regfi_free_record(iter->f, cur);
         /* We're done with this sub-tree, going up and hitting other branches. */
         if(!regfi_iterator_up(iter))
@@ -430 +430 @@
        * Let's move down and print this first sub-tree out. 
        */
-      regfi_free_record(cur);
+      regfi_free_record(iter->f, cur);
       if(!regfi_iterator_down(iter))
       {
@@ -438 +438 @@
 
       cur = regfi_iterator_cur_key(iter);
-      regfi_free_record(sub);
+      regfi_free_record(iter->f, sub);
       regfi_iterator_first_subkey(iter);
       sub = regfi_iterator_cur_subkey(iter);
@@ -445 +445 @@
     printMsgs(iter->f);
   } while(!((cur == root) && (sub == NULL)));
-  regfi_free_record(root);
+  regfi_free_record(iter->f, root);
 
   if(print_verbose)
@@ -518 +518 @@
       printValue(iter, value, tmp_path_joined);
 
-    regfi_free_record(value);
+    regfi_free_record(iter->f, value);
     free(tmp_path);
     free(tmp_path_joined);