You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Fuhrmann <st...@wandisco.com> on 2014/03/02 00:19:00 UTC

Re: Possible unitialised memory in FSX

On Wed, Feb 26, 2014 at 7:13 PM, Philip Martin <ph...@codematters.co.uk>wrote:

> $ svnadmin create repo3 --fs-type fsx
> $  m -f repo3/db/format
> $ printf "1\nlayout sharded 2\n" > repo3/db/format
> $ svn mkdir -mm file://`pwd`/repo3/A
>

Thanks for the reproduction recipe, Philip.


> $ valgrind -q subversion/svnadmin/.libs/lt-svnadmin pack repo3
> ==13523== Syscall param write(buf) points to uninitialised byte(s)
> ==13523==    at 0x4A97110: __write_nocancel (syscall-template.S:82)
> ==13523==    by 0x4A6AAF3: apr_file_flush_locked (readwrite.c:317)
> ==13523==    by 0x4A6B10F: setptr (seek.c:25)
> ==13523==    by 0x4A6B2E5: apr_file_seek (seek.c:70)
> ==13523==    by 0x4111A0F: svn_io_file_seek (io.c:3553)
> ==13523==    by 0x50CB6FA: write_changes_container (pack.c:1490)
> ==13523==    by 0x50CBD28: write_changes_containers (pack.c:1600)
> ==13523==    by 0x50CC904: pack_range (pack.c:1862)
> ==13523==    by 0x50CD246: pack_log_addressed (pack.c:2040)
> ==13523==    by 0x50CD6A3: pack_rev_shard (pack.c:2149)
> ==13523==    by 0x50CD892: pack_shard (pack.c:2204)
> ==13523==    by 0x50CDE78: pack_body (pack.c:2333)
> ==13523==  Address 0x69a5a98 is 56 bytes inside a block of size 4,096
> alloc'd
> ==13523==    at 0x402C1F0: malloc (vg_replace_malloc.c:291)
> ==13523==    by 0x4A6DEEB: pool_alloc (apr_pools.c:1463)
> ==13523==    by 0x4A6E067: apr_palloc_debug (apr_pools.c:1504)
> ==13523==    by 0x4A69573: apr_file_open (open.c:211)
> ==13523==    by 0x410BC09: file_open (io.c:351)
> ==13523==    by 0x411163D: svn_io_file_open (io.c:3428)
> ==13523==    by 0x50C8226: initialize_pack_context (pack.c:261)
> ==13523==    by 0x50CCFDC: pack_log_addressed (pack.c:1996)
> ==13523==    by 0x50CD6A3: pack_rev_shard (pack.c:2149)
> ==13523==    by 0x50CD892: pack_shard (pack.c:2204)
> ==13523==    by 0x50CDE78: pack_body (pack.c:2333)
> ==13523==    by 0x50D9368: with_some_lock_file (transaction.c:250)
> ==13523==
> ==13523== Syscall param write(buf) points to uninitialised byte(s)
> ==13523==    at 0x4A97110: __write_nocancel (syscall-template.S:82)
> ==13523==    by 0x4A6AAF3: apr_file_flush_locked (readwrite.c:317)
> ==13523==    by 0x4A6ABB3: apr_file_flush (readwrite.c:340)
> ==13523==    by 0x4A69252: apr_unix_file_cleanup (open.c:77)
> ==13523==    by 0x4A6F384: apr_pool_cleanup_run (apr_pools.c:2343)
> ==13523==    by 0x4A69698: apr_file_close (open.c:247)
> ==13523==    by 0x41117F5: svn_io_file_close (io.c:3471)
> ==13523==    by 0x50C885F: close_pack_context (pack.c:357)
> ==13523==    by 0x50CD2BF: pack_log_addressed (pack.c:2044)
> ==13523==    by 0x50CD6A3: pack_rev_shard (pack.c:2149)
> ==13523==    by 0x50CD892: pack_shard (pack.c:2204)
> ==13523==    by 0x50CDE78: pack_body (pack.c:2333)
> ==13523==  Address 0x69a6d04 is 68 bytes inside a block of size 4,096
> alloc'd
> ==13523==    at 0x402C1F0: malloc (vg_replace_malloc.c:291)
> ==13523==    by 0x4A6DEEB: pool_alloc (apr_pools.c:1463)
> ==13523==    by 0x4A6E067: apr_palloc_debug (apr_pools.c:1504)
> ==13523==    by 0x4A69573: apr_file_open (open.c:211)
> ==13523==    by 0x410BC09: file_open (io.c:351)
> ==13523==    by 0x411163D: svn_io_file_open (io.c:3428)
> ==13523==    by 0x50BA367: svn_fs_x__l2p_proto_index_open (index.c:415)
> ==13523==    by 0x50C827C: initialize_pack_context (pack.c:266)
> ==13523==    by 0x50CCFDC: pack_log_addressed (pack.c:1996)
> ==13523==    by 0x50CD6A3: pack_rev_shard (pack.c:2149)
> ==13523==    by 0x50CD892: pack_shard (pack.c:2204)
> ==13523==    by 0x50CDE78: pack_body (pack.c:2333)
>

I've got a slightly different stack but it seems to be
the same problem. Took me a while to trace down
the root cause. r1573237 should fix it.

-- Stefan^2.

Re: Possible unitialised memory in FSX

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Sun, Mar 2, 2014 at 7:11 PM, Philip Martin <ph...@wandisco.com>wrote:

> Stefan Fuhrmann <st...@wandisco.com> writes:
>
> > I've got a slightly different stack but it seems to be
> > the same problem. Took me a while to trace down
> > the root cause. r1573237 should fix it.
>
> I'm still seeing one of the warnings:
>
> $ subversion/svnadmin/svnadmin create repo3 --fs-type fsx
> $ rm -rf repo3/db/format
> $ printf "1\nlayout sharded 2\n" > repo3/db/format
> $ subversion/svn/svn mkdir -mm file://`pwd`/repo3/A
>
> $ valgrind -q --suppressions=/home/pm/vg.supp
> subversion/svnadmin/.libs/lt-svnadmin pack repo3
> ==14994== Syscall param write(buf) points to uninitialised byte(s)
> ==14994==    at 0x4A97110: __write_nocancel (syscall-template.S:82)
> ==14994==    by 0x4A6AAF3: apr_file_flush_locked (readwrite.c:317)
> ==14994==    by 0x4A6B10F: setptr (seek.c:25)
> ==14994==    by 0x4A6B2E5: apr_file_seek (seek.c:70)
> ==14994==    by 0x411198F: svn_io_file_seek (io.c:3553)
> ==14994==    by 0x50CCA7A: write_changes_container (pack.c:1490)
> ==14994==    by 0x50CD0A8: write_changes_containers (pack.c:1600)
> ==14994==    by 0x50CDC84: pack_range (pack.c:1862)
> ==14994==    by 0x50CE5C6: pack_log_addressed (pack.c:2040)
> ==14994==    by 0x50CEA23: pack_rev_shard (pack.c:2149)
> ==14994==    by 0x50CEC12: pack_shard (pack.c:2204)
> ==14994==    by 0x50CF1F8: pack_body (pack.c:2333)
> ==14994==  Address 0x69a6a98 is 56 bytes inside a block of size 4,096
> alloc'd
> ==14994==    at 0x402C1F0: malloc (vg_replace_malloc.c:291)
> ==14994==    by 0x4A6DEEB: pool_alloc (apr_pools.c:1463)
> ==14994==    by 0x4A6E067: apr_palloc_debug (apr_pools.c:1504)
> ==14994==    by 0x4A69573: apr_file_open (open.c:211)
> ==14994==    by 0x410BB89: file_open (io.c:351)
> ==14994==    by 0x41115BD: svn_io_file_open (io.c:3428)
> ==14994==    by 0x50C95A6: initialize_pack_context (pack.c:261)
> ==14994==    by 0x50CE35C: pack_log_addressed (pack.c:1996)
> ==14994==    by 0x50CEA23: pack_rev_shard (pack.c:2149)
> ==14994==    by 0x50CEC12: pack_shard (pack.c:2204)
> ==14994==    by 0x50CF1F8: pack_body (pack.c:2333)
> ==14994==    by 0x50DA6E8: with_some_lock_file (transaction.c:250)
> ==14994==
> Packing revisions in shard 0...done.
>
> I'm not familiar with the code, but as far as I can tell it's the line:
>
>   data->len += PADDING; /* there a few extra bytes at then of the buffer
>                            that we want to keep */
>

r1573350 should fix that and the horribly garbled commentary.
Thanks again, Philip.

-- Stefan^2.

Re: Possible unitialised memory in FSX

Posted by Philip Martin <ph...@wandisco.com>.
Stefan Fuhrmann <st...@wandisco.com> writes:

> I've got a slightly different stack but it seems to be
> the same problem. Took me a while to trace down
> the root cause. r1573237 should fix it.

I'm still seeing one of the warnings:

$ subversion/svnadmin/svnadmin create repo3 --fs-type fsx
$ rm -rf repo3/db/format
$ printf "1\nlayout sharded 2\n" > repo3/db/format
$ subversion/svn/svn mkdir -mm file://`pwd`/repo3/A

$ valgrind -q --suppressions=/home/pm/vg.supp subversion/svnadmin/.libs/lt-svnadmin pack repo3
==14994== Syscall param write(buf) points to uninitialised byte(s)
==14994==    at 0x4A97110: __write_nocancel (syscall-template.S:82)
==14994==    by 0x4A6AAF3: apr_file_flush_locked (readwrite.c:317)
==14994==    by 0x4A6B10F: setptr (seek.c:25)
==14994==    by 0x4A6B2E5: apr_file_seek (seek.c:70)
==14994==    by 0x411198F: svn_io_file_seek (io.c:3553)
==14994==    by 0x50CCA7A: write_changes_container (pack.c:1490)
==14994==    by 0x50CD0A8: write_changes_containers (pack.c:1600)
==14994==    by 0x50CDC84: pack_range (pack.c:1862)
==14994==    by 0x50CE5C6: pack_log_addressed (pack.c:2040)
==14994==    by 0x50CEA23: pack_rev_shard (pack.c:2149)
==14994==    by 0x50CEC12: pack_shard (pack.c:2204)
==14994==    by 0x50CF1F8: pack_body (pack.c:2333)
==14994==  Address 0x69a6a98 is 56 bytes inside a block of size 4,096 alloc'd
==14994==    at 0x402C1F0: malloc (vg_replace_malloc.c:291)
==14994==    by 0x4A6DEEB: pool_alloc (apr_pools.c:1463)
==14994==    by 0x4A6E067: apr_palloc_debug (apr_pools.c:1504)
==14994==    by 0x4A69573: apr_file_open (open.c:211)
==14994==    by 0x410BB89: file_open (io.c:351)
==14994==    by 0x41115BD: svn_io_file_open (io.c:3428)
==14994==    by 0x50C95A6: initialize_pack_context (pack.c:261)
==14994==    by 0x50CE35C: pack_log_addressed (pack.c:1996)
==14994==    by 0x50CEA23: pack_rev_shard (pack.c:2149)
==14994==    by 0x50CEC12: pack_shard (pack.c:2204)
==14994==    by 0x50CF1F8: pack_body (pack.c:2333)
==14994==    by 0x50DA6E8: with_some_lock_file (transaction.c:250)
==14994== 
Packing revisions in shard 0...done.

I'm not familiar with the code, but as far as I can tell it's the line:

  data->len += PADDING; /* there a few extra bytes at then of the buffer
                           that we want to keep */

in create_table() which extends the sizeof the stringbuf by 8 bytes to
10 bytes total and nothing ever writes to those extra bytes.  The
following patch removes the warning, demonstrating that it those bytes
that are triggering it:

Index: subversion/libsvn_fs_x/string_table.c
===================================================================
--- subversion/libsvn_fs_x/string_table.c	(revision 1573320)
+++ subversion/libsvn_fs_x/string_table.c	(working copy)
@@ -384,6 +384,9 @@ create_table(string_sub_table_t *target,
     = svn_stringbuf_create_ensure(MAX_DATA_SIZE - source->max_data_size,
                                   scratch_pool);
 
+  svn_stringbuf_appendfill(data, 'x', 10);
+  svn_stringbuf_setempty(data);
+
   /* pack sub-strings */
   target->short_string_count = (apr_size_t)source->short_strings->nelts;
   target->short_strings = apr_palloc(pool, sizeof(*target->short_strings) *


-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*