[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Possible bug in libsvn_ra_dav/commit.c? Any sprintf gurus please look.

From: Branko Čibej <brane_at_xbc.nu>
Date: 2001-11-21 18:13:11 CET

B. W. Fitzpatrick wrote:

>(Questions at the very end. This is just a lengthy lead-in). :)
>
>OK. I've spent the better part of this evening trying to figure this
>out. 'svn commit' works fine... on every platform except Mac OS X.
>
>(In other news, I got svn building on Mac OS X 10.1.0
>tonight. Yaaaay. Now I need to get it to work!).
>
>When I run 'svn commit', I get a Segmentation Fault on Mac OS X.
>
>I've run svn under gdb eight ways to Sunday, and I've isolated the
>crash to libsvn_ra_dav/commit.c, in the function create_activity.
>
>Now here's where it gets weird.
>
>For reference, here's the code in question:
>
>1 static svn_error_t * create_activity(commit_ctx_t *cc)
>2 {
>3 svn_stringbuf_t * activity_url;
>4 apr_uuid_t uuid;
>5 char uuid_buf[APR_UUID_FORMATTED_LENGTH];
>6 svn_stringbuf_t uuid_str = { uuid_buf, sizeof(uuid_buf), 0, NULL };
>7 int code;
>8
>9 /* get the URL where we'll create activities */
>10 SVN_ERR( get_activity_url(cc, &activity_url) );
>11
>12 /* the URL for our activity will be ACTIVITY_URL/UUID */
>13 apr_uuid_get(&uuid);
>14 apr_uuid_format(uuid_buf, &uuid);
>15
>16 /* ### grumble. this doesn't watch out for trailing "/" */
>17 svn_path_add_component(activity_url, &uuid_str, svn_path_url_style);
>...
>
>Note that uuid_buf is declared (APR_UUID_FORMATTED_LENGTH == 36) right
>before uuid_str, the first member of which is a pointer to the
>beginning of the uuid_buf char array.
>
>now, if I do a 'p uuid_str' with my program pointer on line 14 (before
>apr_uuid_format is called), I get this:
>
> (gdb) p uuid_str
> $3 = {
> data = 0xbffff9bc "",
> len = 36,
> blocksize = 0,
> pool = 0x0
> }
>
>However, if I step over that and do the same on line 15, I get this:
>
> (gdb) p uuid_str
> $6 = {
> data = 0xfff9bc <Address 0xfff9bc out of bounds>,
> len = 36,
> blocksize = 0,
> pool = 0x0
> }
>
>the pointer in data has changed from:
>
> 0xbffff9bc to
> 0xfff9bc
>
>Looks to me like the beginning of the pointer is getting truncated.
>

Oh &%@#!

You know, this may be the same thing that's killing the Win32 Release build.

Good catch!

>My guess is that the sprintf is writing 37 bytes starting from
>uuid_buf, and uuid_str->data, being next in memory, is getting stomped
>by the trailing null. 'man sprintf' was a bit confusing, but this is
>what I get from it...
>
>So I look inside of apr_uuid_format. Basically it does an sprintf of
>uuid->data into uuid_buf. I checked... it's doing sprintf'ing 36
>bytes, and we've allocated 36 bytes, so all should be OK.
>
>And now, finally my questions:
>
>1. Doesn't sprintf throw an \0 onto the end of the string that you sprintf into?
>
Yes, it does.

>2. Shouldn't we allocate uuid_buf as [APR_UUID_FORMATTED_LENGTH +1] ?
>
Yes, we should.

>3. Why doesn't this blow up on other platforms? Why oh why oh why?
>
It depends on stack alignment and padding, so it depends on the system
ABI, compiler options and sometimes on sunspot activity.

>4. When it comes to stewed prunes, is three enough? Is two too many?
>
Depends on the size of the prunes, of course. :-)

> Any insight would be appreciated so that I can fix this bug.

Just allocate APR_UUID_FORMATTED_LENGTH +1. That should fix it, I guess.

>If any apr folk on this list have access to moof.apache.org, you'll
>find svn in my home dir if you want to check it out.
>

-- 
Brane �ibej   <brane_at_xbc.nu>   http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:49 2006

This is an archived mail posted to the Subversion Dev mailing list.