You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2002/05/14 22:44:08 UTC
unitialised memory read, a suspicious MD5 checksum?
Hello
I've been playing with valgrind, a run-time memory checker. It's
impressive when it works! I'm getting an unitialised memory read in
the function skel.c:use_implict(). I tracked it down to this scenario
svnadmin create repo
svn co -d wc file:///home/pm/repo
svn ps a 1 wc
svn ci -m "" wc
svn up wc
svn ps b 1 wc
svn ci -m "" wc
svn up wc
svn ps c 1 wc
svn ci -m "" wc
svn up wc
svn ps d 1 wc
The first three commits are successful. When I attempt the fourth
valgrind warns about the line:
==27941== Use of uninitialised value of size 4
==27941== at 0x403540CD: use_implicit (../svn/subversion/libsvn_fs/skel.c:328)
==27941== by 0x40354187: unparse (../svn/subversion/libsvn_fs/skel.c:356)
==27941== by 0x403542BA: unparse (../svn/subversion/libsvn_fs/skel.c:389)
==27941== by 0x403542BA: unparse (../svn/subversion/libsvn_fs/skel.c:389)
In gdb I see
#2 0x403540cd in use_implicit (skel=0x4342ca48)
at ../svn/subversion/libsvn_fs/skel.c:328
328 if (skel_char_type[(unsigned char) skel->data[0]] != type_name)
(gdb) p skel[0]
$10 = {is_atom = 1, data = 0x4342c97c "", len = 16, children = 0x0, next = 0x0}
After a little investigation it appears to be a suspicious md5
checksum. Running the commit without valgrind and setting a breakpoint
in svn_fs__unparse_representation_skel at fs_skels.c:668 I see
668 svn_fs__prepend (svn_fs__str_atom ("svndiff", pool), diff_skel);
(gdb) p chunk[0]
$1 = {offset = 0, string_key = 0x405d0158 "6", size = 19,
checksum = '\0' <repeats 15 times>, rep_key = 0x8092950 "3"}
(gdb) p chunks->nelts
$3 = 1
The code in fs_skels.c is
svn_fs__prepend (svn_fs__mem_atom (chunk->checksum,
MD5_DIGESTSIZE /
sizeof (*(chunk->checksum)),
pool), checksum_skel);
svn_fs__prepend (svn_fs__str_atom ("md5", pool), checksum_skel);
so it is about to create an md5 skel with a checksum that is all null
bytes. This looks suspicious to me. It has been called from
#0 svn_fs__unparse_representation_skel (skel_p=0xbffff498, rep=0xbffff4f4,
pool=0x80921e8) at ../svn/subversion/libsvn_fs/fs_skels.c:671
#1 0x40073c47 in svn_fs__write_rep (fs=0x8060d50, key=0x8092860 "2",
rep=0xbffff4f4, trail=0x806eef8)
at ../svn/subversion/libsvn_fs/reps-table.c:113
#2 0x4007366b in svn_fs__rep_deltify (fs=0x8060d50, target=0x8092860 "2",
source=0x8092950 "3", trail=0x806eef8)
at ../svn/subversion/libsvn_fs/reps-strings.c:1452
#3 0x4006ba9c in deltify (target_id=0x80902e8, source_id=0x8092618,
fs=0x8060d50, props_only=1, trail=0x806eef8)
at ../svn/subversion/libsvn_fs/deltify.c:60
#4 0x4006bde2 in deltify_by_id (fs=0x8060d50, target_id=0x80902e8, is_dir=1,
trail=0x806eef8) at ../svn/subversion/libsvn_fs/deltify.c:155
#5 0x4006bfdb in deltify_undeltify (fs=0x8060d50, root=0x808e2a8,
path=0x4007f62a "/", id=0x80902e8, do_deltify=1, recurse=1,
trail=0x806eef8) at ../svn/subversion/libsvn_fs/deltify.c:219
#6 0x4006c0d8 in txn_body_deltify (baton=0xbffff6f8, trail=0x806eef8)
at ../svn/subversion/libsvn_fs/deltify.c:255
#7 0x40076a95 in svn_fs__retry_txn (fs=0x8060d50,
txn_body=0x4006c03c <txn_body_deltify>, baton=0xbffff6f8, pool=0x806ed80)
at ../svn/subversion/libsvn_fs/trail.c:127
#8 0x4006c2a1 in svn_fs_deltify (root=0x808e2a8, path=0x4007f62a "/",
recursive=1, pool=0x806ed80) at ../svn/subversion/libsvn_fs/deltify.c:304
#9 0x4007990b in svn_fs_commit_txn (conflict_p=0xbffff7fc,
new_rev=0xbffff804, txn=0x806ee50)
at ../svn/subversion/libsvn_fs/tree.c:1895
I don't know much about skels, the fs layer, or md5 checksums. In
addition valgrind is very much still under development. Is this a
false alarm from valgrind or is the chunk/checksum wrong?
--
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: unitialised memory read, a suspicious MD5 checksum?
Posted by cm...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> cmpilato@collab.net writes:
>
> > Philip Martin <ph...@codematters.co.uk> writes:
> >
> > > After a little investigation it appears to be a suspicious md5
> > > checksum. Running the commit without valgrind and setting a breakpoint
> > > in svn_fs__unparse_representation_skel at fs_skels.c:668 I see
> >
> > Yeah, we simply aren't using the MD5 checksum field right now -- it's
> > one of the many things on the "to-do" list.
>
> Ok, I still think we might have a bug in the current code.
>
> Looking at svn_fs__rep_deltify() it calls svn_txdelta() to create
> txdelta_stream. This does not initialise the digest member. Later
> svn_fs__rep_deltify() calls svn_txdelta_md5_digest() to retrieve the
> digest, it has still not been initialised. It gets memcpy'd into a
> chunk, and later memcpy'd into a skel. This is the memory that is
> used in use_implict() to query the skel_char_type table.
Okay, tracing through this code now. svn_txdelta() does indeed create
an initialized `digest' member, though that digest should be in safely
allocated memory (just full of garbage). From there on, that digest
is only used in two ways: copied to other digests, and marshaled into
and out of skels.
The marshaling is where you're seeing code that is making decisions
based on the contents of `digest', but this is far from dangerous -- I
mean, a *real* MD5 digest would suffer the same way, more or less
depending on how the bits fall. Think of use_implicit as a way of
choosing whether or not to base64 encode or uuencode some data -- it
doesn't really matter the decision made, because the original data
(garbage, in this case) can still be unmarshaled later (at which time
it will be used for nothing).
So, this uninitialized data should not be dangerous, but is an
annoyance that causes warnings in your boundschecker. I'd say just
use apr_pcalloc (instead of apr_palloc) in svn_txdelta() and be done
with it. :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: unitialised memory read, a suspicious MD5 checksum?
Posted by Philip Martin <ph...@codematters.co.uk>.
cmpilato@collab.net writes:
> Philip Martin <ph...@codematters.co.uk> writes:
>
> > After a little investigation it appears to be a suspicious md5
> > checksum. Running the commit without valgrind and setting a breakpoint
> > in svn_fs__unparse_representation_skel at fs_skels.c:668 I see
>
> Yeah, we simply aren't using the MD5 checksum field right now -- it's
> one of the many things on the "to-do" list.
Ok, I still think we might have a bug in the current code.
Looking at svn_fs__rep_deltify() it calls svn_txdelta() to create
txdelta_stream. This does not initialise the digest member. Later
svn_fs__rep_deltify() calls svn_txdelta_md5_digest() to retrieve the
digest, it has still not been initialised. It gets memcpy'd into a
chunk, and later memcpy'd into a skel. This is the memory that is
used in use_implict() to query the skel_char_type table.
Thus the arbitrary unitialised value of digest in svn_txdelta()
determines whether use_implict() returns 1 or 0. (I checked, using
the debugger, whatever value I put into digest in svn_txdelta()
appears when use_implict() is called.) This looks wrong to me, but I
don't know enough about the code to say what the initial value of
digest should be.
--
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: unitialised memory read, a suspicious MD5 checksum?
Posted by cm...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> After a little investigation it appears to be a suspicious md5
> checksum. Running the commit without valgrind and setting a breakpoint
> in svn_fs__unparse_representation_skel at fs_skels.c:668 I see
Yeah, we simply aren't using the MD5 checksum field right now -- it's
one of the many things on the "to-do" list.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org