svn commit: r1383180 - /subversion/trunk/subversion/libsvn_subr/cache-membuffer.c

[st...@apache.org]

Author: stefan2
Date: Tue Sep 11 00:18:50 2012
New Revision: 1383180

URL: http://svn.apache.org/viewvc?rev=1383180&view=rev
Log:
Fix a subtle buffer overflow in the membuffer cache when the data buffer
size is not a multiple of 16 bytes (ITEM_ALIGNMENT).  Because we will
move the insertion pointer always in multiples of ITEM_ALIGMENT, it could
then be moved to a position behind the buffer where capacity - position < 0.

Also, fix two small style and spelling issues.

* subversion/libsvn_subr/cache-membuffer.c
  (insert_entry, move_entry): add assertions to debug overflows in future
  (ensure_data_insertable): make condition immune to overflows
  (svn_cache__membuffer_cache_create): actual fix
  (drop_entry): minor formatting fix
  (membuffer_cache_set_internal): fix typo

Modified:
    subversion/trunk/subversion/libsvn_subr/cache-membuffer.c

Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1383180&r1=1383179&r2=1383180&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Tue Sep 11 00:18:50 2012
@@ -576,7 +576,7 @@ drop_entry(svn_membuffer_t *cache, entry
             /* remove the first entry -> insertion may start at pos 0, now */
             cache->current_data = 0;
           }
-          else
+        else
           {
             /* insertion may start right behind the previous entry */
             entry_t *previous = get_entry(cache, entry->previous);
@@ -658,6 +658,11 @@ insert_entry(svn_membuffer_t *cache, ent
       else
         cache->first = idx;
     }
+
+  /* The current insertion position must never point outside our
+   * data buffer.
+   */
+  assert(cache->current_data <= cache->data_size);
 }
 
 /* Map a KEY of 16 bytes to the CACHE and group that shall contain the
@@ -857,6 +862,11 @@ move_entry(svn_membuffer_t *cache, entry
    */
   cache->current_data = entry->offset + size;
   cache->next = entry->next;
+
+  /* The current insertion position must never point outside our
+   * data buffer.
+   */
+  assert(cache->current_data <= cache->data_size);
 }
 
 /* If necessary, enlarge the insertion window until it is at least
@@ -898,7 +908,7 @@ ensure_data_insertable(svn_membuffer_t *
 
       /* leave function as soon as the insertion window is large enough
        */
-      if (end - cache->current_data >= size)
+      if (end >= size + cache->current_data)
         return TRUE;
 
       /* Don't be too eager to cache data. Smaller items will fit into
@@ -1055,8 +1065,10 @@ svn_cache__membuffer_cache_create(svn_me
 
   /* limit the data size to what we can address.
    * Note that this cannot overflow since all values are of size_t.
+   * Also, make it a multiple of the item placement granularity to
+   * prevent subtle overflows.
    */
-  data_size = total_size - directory_size;
+  data_size = ALIGN_VALUE(total_size - directory_size + 1) - ITEM_ALIGNMENT;
 
   /* For cache sizes > 4TB, individual cache segments will be larger
    * than 16GB allowing for >4GB entries.  But caching chunks larger
@@ -1190,7 +1202,7 @@ membuffer_cache_set_internal(svn_membuff
         {
           /* Remove old data for this key, if that exists.
            * Get an unused entry for the key and and initialize it with
-           * the serialized item's (future) posion within data buffer.
+           * the serialized item's (future) position within data buffer.
            */
           entry = find_entry(cache, group_index, to_find, TRUE);
           entry->size = size;