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 2014/08/07 23:35:02 UTC
svn commit: r1616600 - in /subversion/trunk/subversion:
include/svn_error_codes.h libsvn_fs_x/pack.c libsvn_fs_x/reps.c
libsvn_fs_x/reps.h tests/libsvn_fs_x/fs-x-pack-test.c
Author: stefan2
Date: Thu Aug 7 21:35:01 2014
New Revision: 1616600
URL: http://svn.apache.org/r1616600
Log:
Explicitly limit the dimensions of a star delta container which allows us
to cast sizes to 32 bits.
* subversion/include/svn_error_codes.h
(SVN_ERR_FS_CONTAINER_SIZE): Declare new error code.
* subversion/libsvn_fs_x/reps.h
(svn_fs_x__reps_add): Allow for error returns.
* subversion/libsvn_fs_x/reps.c
(MAX_TEXT_BODY,
MAX_INSTRUCTIONS): Formally declare dimensional limit to the star
delta container.
(svn_fs_x__reps_add_base): Update caller. Explicitly cast index to 32 bits.
(add_new_text): Explicitly cast length values to 32 bits. They have been
limited to < 2GB be the caller already.
(svn_fs_x__reps_add): Make sure the container size stays within set limits.
* subversion/libsvn_fs_x/pack.c
(write_reps_containers): Update caller.
* subversion/tests/libsvn_fs_x/fs-x-pack-test.c
(test_reps): Same.
Modified:
subversion/trunk/subversion/include/svn_error_codes.h
subversion/trunk/subversion/libsvn_fs_x/pack.c
subversion/trunk/subversion/libsvn_fs_x/reps.c
subversion/trunk/subversion/libsvn_fs_x/reps.h
subversion/trunk/subversion/tests/libsvn_fs_x/fs-x-pack-test.c
Modified: subversion/trunk/subversion/include/svn_error_codes.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_error_codes.h?rev=1616600&r1=1616599&r2=1616600&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_error_codes.h (original)
+++ subversion/trunk/subversion/include/svn_error_codes.h Thu Aug 7 21:35:01 2014
@@ -851,6 +851,11 @@ SVN_ERROR_START
SVN_ERR_FS_CATEGORY_START + 60,
"Unsupported FS type")
+ /** @since New in 1.9. */
+ SVN_ERRDEF(SVN_ERR_FS_CONTAINER_SIZE,
+ SVN_ERR_FS_CATEGORY_START + 61,
+ "Container capacity exceeded.")
+
/* repos errors */
SVN_ERRDEF(SVN_ERR_REPOS_LOCKED,
Modified: subversion/trunk/subversion/libsvn_fs_x/pack.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/pack.c?rev=1616600&r1=1616599&r2=1616600&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/pack.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/pack.c Thu Aug 7 21:35:01 2014
@@ -1343,8 +1343,8 @@ write_reps_containers(pack_context_t *co
SVN_ERR(svn_stream_read_full(stream, contents->data, &contents->len));
SVN_ERR(svn_stream_close(stream));
- list_index = svn_fs_x__reps_add(container,
- svn_stringbuf__morph_into_string(contents));
+ SVN_ERR(svn_fs_x__reps_add(&list_index, container,
+ svn_stringbuf__morph_into_string(contents)));
SVN_ERR_ASSERT(list_index == sub_items->nelts);
block_left -= entry->size;
Modified: subversion/trunk/subversion/libsvn_fs_x/reps.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/reps.c?rev=1616600&r1=1616599&r2=1616600&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/reps.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/reps.c Thu Aug 7 21:35:01 2014
@@ -27,6 +27,8 @@
#include "private/svn_packed_data.h"
#include "private/svn_temp_serializer.h"
+#include "svn_private_config.h"
+
#include "cached_data.h"
/* Length of the text chunks we hash and match. The algorithm will find
@@ -38,6 +40,16 @@
*/
#define MATCH_BLOCKSIZE 64
+/* Limit the total text body within a container to 16MB. Larger values
+ * of up to 2GB are possible but become increasingly impractical as the
+ * container has to be loaded in its entirety before any of it can be read.
+ */
+#define MAX_TEXT_BODY 0x1000000
+
+/* Limit the size of the instructions stream. This should not exceed the
+ * text body size limit. */
+#define MAX_INSTRUCTIONS (MAX_TEXT_BODY / 8)
+
/* value of unused hash buckets */
#define NO_OFFSET ((apr_uint32_t)(-1))
@@ -398,15 +410,17 @@ svn_fs_x__reps_add_base(svn_fs_x__reps_b
svn_stream_t *stream;
svn_string_t *contents;
+ apr_size_t idx;
SVN_ERR(svn_fs_x__get_contents(&stream, builder->fs, rep, FALSE,
scratch_pool));
SVN_ERR(svn_string_from_stream(&contents, stream, scratch_pool,
scratch_pool));
+ SVN_ERR(svn_fs_x__reps_add(&idx, builder, contents));
base.revision = svn_fs_x__get_revnum(rep->id.change_set);
base.item_index = rep->id.number;
base.priority = priority;
- base.rep = (apr_uint32_t)svn_fs_x__reps_add(builder, contents);
+ base.rep = (apr_uint32_t)idx;
APR_ARRAY_PUSH(builder->bases, base_t) = base;
builder->base_text_len += builder->text->len - text_start_offset;
@@ -430,8 +444,8 @@ add_new_text(svn_fs_x__reps_builder_t *b
return;
/* new instruction */
- instruction.offset = builder->text->len;
- instruction.count = len;
+ instruction.offset = (apr_int32_t)builder->text->len;
+ instruction.count = (apr_uint32_t)len;
APR_ARRAY_PUSH(builder->instructions, instruction_t) = instruction;
/* add to text corpus */
@@ -462,8 +476,9 @@ add_new_text(svn_fs_x__reps_builder_t *b
}
}
-apr_size_t
-svn_fs_x__reps_add(svn_fs_x__reps_builder_t *builder,
+svn_error_t *
+svn_fs_x__reps_add(apr_size_t *rep_idx,
+ svn_fs_x__reps_builder_t *builder,
const svn_string_t *contents)
{
rep_t rep;
@@ -472,8 +487,16 @@ svn_fs_x__reps_add(svn_fs_x__reps_builde
const char *end = current + contents->len;
const char *last_to_test = end - MATCH_BLOCKSIZE - 1;
- rep.first_instruction = (apr_uint32_t)builder->instructions->nelts;
+ if (builder->text->len + contents->len > MAX_TEXT_BODY)
+ return svn_error_create(SVN_ERR_FS_CONTAINER_SIZE, NULL,
+ _("Text body exceeds star delta container capacity"));
+
+ if ( builder->instructions->nelts + 2 * contents->len / MATCH_BLOCKSIZE
+ > MAX_INSTRUCTIONS)
+ return svn_error_create(SVN_ERR_FS_CONTAINER_SIZE, NULL,
+ _("Instruction count exceeds star delta container capacity"));
+ rep.first_instruction = (apr_uint32_t)builder->instructions->nelts;
while (current < last_to_test)
{
hash_key_t key = hash_key(current);
@@ -501,7 +524,7 @@ svn_fs_x__reps_add(svn_fs_x__reps_builde
if (current < last_to_test)
{
instruction_t instruction;
-
+
/* extend the match */
size_t prefix_match
@@ -536,7 +559,8 @@ svn_fs_x__reps_add(svn_fs_x__reps_builde
- rep.first_instruction;
APR_ARRAY_PUSH(builder->reps, rep_t) = rep;
- return (apr_size_t)(builder->reps->nelts - 1);
+ *rep_idx = (apr_size_t)(builder->reps->nelts - 1);
+ return SVN_NO_ERROR;
}
apr_size_t
Modified: subversion/trunk/subversion/libsvn_fs_x/reps.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/reps.h?rev=1616600&r1=1616599&r2=1616600&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/reps.h (original)
+++ subversion/trunk/subversion/libsvn_fs_x/reps.h Thu Aug 7 21:35:01 2014
@@ -96,10 +96,11 @@ svn_fs_x__reps_add_base(svn_fs_x__reps_b
apr_pool_t *scratch_pool);
/* Add the byte string CONTENTS to BUILDER. Return the item index under
- * which the fulltext can be retrieved from the final container.
+ * which the fulltext can be retrieved from the final container in *REP_IDX.
*/
-apr_size_t
-svn_fs_x__reps_add(svn_fs_x__reps_builder_t *builder,
+svn_error_t *
+svn_fs_x__reps_add(apr_size_t *rep_idx,
+ svn_fs_x__reps_builder_t *builder,
const svn_string_t *contents);
/* Return a rough estimate in bytes for the serialized representation
Modified: subversion/trunk/subversion/tests/libsvn_fs_x/fs-x-pack-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs_x/fs-x-pack-test.c?rev=1616600&r1=1616599&r2=1616600&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs_x/fs-x-pack-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs_x/fs-x-pack-test.c Thu Aug 7 21:35:01 2014
@@ -814,11 +814,12 @@ test_reps(const svn_test_opts_t *opts,
builder = svn_fs_x__reps_builder_create(fs, pool);
for (i = 10000; i > 10; --i)
{
+ apr_size_t idx;
svn_string_t string;
string.data = contents->data;
string.len = i;
- svn_fs_x__reps_add(builder, &string);
+ SVN_ERR(svn_fs_x__reps_add(&idx, builder, &string));
}
serialized = svn_stringbuf_create_empty(pool);
Re: svn commit: r1616600 - in /subversion/trunk/subversion:
include/svn_error_codes.h libsvn_fs_x/pack.c libsvn_fs_x/reps.c
libsvn_fs_x/reps.h tests/libsvn_fs_x/fs-x-pack-test.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Fri, Aug 8, 2014 at 12:08 AM, Branko Čibej <br...@wandisco.com> wrote:
> On 08.08.2014 00:05, Stefan Fuhrmann wrote:
>
>
> On Thu, Aug 7, 2014 at 11:49 PM, Branko Čibej <br...@wandisco.com> wrote:
>
>> On 07.08.2014 23:35, stefan2@apache.org wrote:
>>
>> Author: stefan2
>> Date: Thu Aug 7 21:35:01 2014
>> New Revision: 1616600
>>
>> URL: http://svn.apache.org/r1616600
>> Log:
>> Explicitly limit the dimensions of a star delta container which allows us
>> to cast sizes to 32 bits.
>>
>>
>> Can we explicitly use 32-bit types then, pretty please? Peppering the
>> code with casts is not the way to silence compiler warnings. Using correct
>> types is.
>>
>
> You mean like svn_stringbuf32_t?
>
> All FSX internal data structures in use explicit bitness (or are
> supposed to do so) to prevent unpleasant surprises when
> downgrading from 64 to 32 bit server software.
>
> However, at some point we have to cast from the platform
> types we use in our aux data structures to the internal ones.
>
>
> Ah, you mean adapting to the FS API.
>
Not only that but also that we use svn_stringbuf_t and similar
data structures / API from lib_subr where they make sense.
-- Stefan^2.
Re: svn commit: r1616600 - in /subversion/trunk/subversion: include/svn_error_codes.h
libsvn_fs_x/pack.c libsvn_fs_x/reps.c libsvn_fs_x/reps.h tests/libsvn_fs_x/fs-x-pack-test.c
Posted by Branko Čibej <br...@wandisco.com>.
On 08.08.2014 00:05, Stefan Fuhrmann wrote:
>
>
>
> On Thu, Aug 7, 2014 at 11:49 PM, Branko Čibej <brane@wandisco.com
> <ma...@wandisco.com>> wrote:
>
> On 07.08.2014 23:35, stefan2@apache.org
> <ma...@apache.org> wrote:
>> Author: stefan2
>> Date: Thu Aug 7 21:35:01 2014
>> New Revision: 1616600
>>
>> URL: http://svn.apache.org/r1616600
>> Log:
>> Explicitly limit the dimensions of a star delta container which allows us
>> to cast sizes to 32 bits.
>
> Can we explicitly use 32-bit types then, pretty please? Peppering
> the code with casts is not the way to silence compiler warnings.
> Using correct types is.
>
>
> You mean like svn_stringbuf32_t?
>
> All FSX internal data structures in use explicit bitness (or are
> supposed to do so) to prevent unpleasant surprises when
> downgrading from 64 to 32 bit server software.
>
> However, at some point we have to cast from the platform
> types we use in our aux data structures to the internal ones.
Ah, you mean adapting to the FS API.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane@wandisco.com
Re: svn commit: r1616600 - in /subversion/trunk/subversion:
include/svn_error_codes.h libsvn_fs_x/pack.c libsvn_fs_x/reps.c
libsvn_fs_x/reps.h tests/libsvn_fs_x/fs-x-pack-test.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Thu, Aug 7, 2014 at 11:49 PM, Branko Čibej <br...@wandisco.com> wrote:
> On 07.08.2014 23:35, stefan2@apache.org wrote:
>
> Author: stefan2
> Date: Thu Aug 7 21:35:01 2014
> New Revision: 1616600
>
> URL: http://svn.apache.org/r1616600
> Log:
> Explicitly limit the dimensions of a star delta container which allows us
> to cast sizes to 32 bits.
>
>
> Can we explicitly use 32-bit types then, pretty please? Peppering the code
> with casts is not the way to silence compiler warnings. Using correct types
> is.
>
You mean like svn_stringbuf32_t?
All FSX internal data structures in use explicit bitness (or are
supposed to do so) to prevent unpleasant surprises when
downgrading from 64 to 32 bit server software.
However, at some point we have to cast from the platform
types we use in our aux data structures to the internal ones.
-- Stefan^2.
Re: svn commit: r1616600 - in /subversion/trunk/subversion: include/svn_error_codes.h
libsvn_fs_x/pack.c libsvn_fs_x/reps.c libsvn_fs_x/reps.h tests/libsvn_fs_x/fs-x-pack-test.c
Posted by Branko Čibej <br...@wandisco.com>.
On 07.08.2014 23:35, stefan2@apache.org wrote:
> Author: stefan2
> Date: Thu Aug 7 21:35:01 2014
> New Revision: 1616600
>
> URL: http://svn.apache.org/r1616600
> Log:
> Explicitly limit the dimensions of a star delta container which allows us
> to cast sizes to 32 bits.
Can we explicitly use 32-bit types then, pretty please? Peppering the
code with casts is not the way to silence compiler warnings. Using
correct types is.
That might require adding number pack/unpack functions that work with
correctly sized types, but since the idea is to detect overflow in the
numbers, this is actually a good idea (and does not affect the packed
representation of the numbers).
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane@wandisco.com