You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2015/11/30 22:52:29 UTC

svn commit: r1717334 - in /subversion/trunk/subversion/libsvn_fs_fs: cached_data.c index.c revprops.c

Author: stefan2
Date: Mon Nov 30 21:52:29 2015
New Revision: 1717334

URL: http://svn.apache.org/viewvc?rev=1717334&view=rev
Log:
Complete overflow checking working in FSFS.

While the principle is the same as in r1714372 and r1717332, the changes
here were less straight-forward.

* subversion/libsvn_fs_fs/cached_data.c
  (get_contents_from_windows): Rewrite the limiter code such that no
                               arithmetic overflow may occur.

* subversion/libsvn_fs_fs/index.c
  (read_entry): Add another overflow check to ensure that the frequently
                used "offset + size" expression never causes an overflow.

* subversion/libsvn_fs_fs/revprops.c
  (svn_fs_fs__pack_revprops_shard): Extend the overflow check to cover all
                                    theoretical edge cases.

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
    subversion/trunk/subversion/libsvn_fs_fs/index.c
    subversion/trunk/subversion/libsvn_fs_fs/revprops.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/cached_data.c?rev=1717334&r1=1717333&r2=1717334&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/cached_data.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/cached_data.c Mon Nov 30 21:52:29 2015
@@ -1801,10 +1801,10 @@ get_contents_from_windows(struct rep_rea
              This is where we need the pseudo rep_state created
              by build_rep_list(). */
           apr_size_t offset = (apr_size_t)rs->current;
-          if (copy_len + offset > rb->base_window->len)
-            copy_len = offset < rb->base_window->len
-                     ? rb->base_window->len - offset
-                     : 0ul;
+          if (offset >= rb->base_window->len)
+            copy_len = 0ul;
+          else if (copy_len > rb->base_window->len - offset)
+            copy_len = rb->base_window->len - offset;
 
           memcpy (cur, rb->base_window->data + offset, copy_len);
         }

Modified: subversion/trunk/subversion/libsvn_fs_fs/index.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/index.c?rev=1717334&r1=1717333&r2=1717334&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/index.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/index.c Mon Nov 30 21:52:29 2015
@@ -2422,6 +2422,13 @@ read_entry(svn_fs_fs__packed_number_stre
       return svn_error_create(SVN_ERR_FS_INDEX_CORRUPTION, NULL,
                  _("Empty regions must have item number 0 and checksum 0"));
 
+  /* Corrupted SIZE values might cause arithmetic overflow.
+   * The same can happen if you copy a repository from a system with 63 bit
+   * file lengths to one with 31 bit file lengths. */
+  if ((apr_uint64_t)entry.offset + (apr_uint64_t)entry.size > off_t_max)
+    return svn_error_create(SVN_ERR_FS_INDEX_OVERFLOW , NULL,
+                            _("P2L index entry size overflow."));
+
   APR_ARRAY_PUSH(result, svn_fs_fs__p2l_entry_t) = entry;
   *item_offset += entry.size;
 

Modified: subversion/trunk/subversion/libsvn_fs_fs/revprops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/revprops.c?rev=1717334&r1=1717333&r2=1717334&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/revprops.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/revprops.c Mon Nov 30 21:52:29 2015
@@ -1377,10 +1377,13 @@ svn_fs_fs__pack_revprops_shard(const cha
                              iterpool);
       SVN_ERR(svn_io_stat(&finfo, path, APR_FINFO_SIZE, iterpool));
 
-      /* if we already have started a pack file and this revprop cannot be
-       * appended to it, write the previous pack file. */
-      if (sizes->nelts != 0 &&
-          total_size + SVN_INT64_BUFFER_SIZE + finfo.size > max_size)
+      /* If we already have started a pack file and this revprop cannot be
+       * appended to it, write the previous pack file.  Note this overflow
+       * check works because we enforced MAX_SIZE <= SVN_MAX_OBJECT_SIZE. */
+      if (sizes->nelts != 0
+          && (   finfo.size > max_size
+              || total_size > max_size
+              || SVN_INT64_BUFFER_SIZE + finfo.size > max_size - total_size))
         {
           SVN_ERR(svn_fs_fs__copy_revprops(pack_file_dir, pack_filename,
                                            shard_path, start_rev, rev-1,