On Wed, Jan 21, 2009 at 04:02:54PM +0100, Greg Stein wrote:
> 0x69737265 looks like ascii characters: i s r e
>
> Somehow the path pointer is getting munged by some text?
We need to audit that code more.
This seems to be yet another C-string parsing pointer bug.
Quite a few have already been found and fixed, see the history
of libsvn_subr/dirent_uri.c
I've already gone through the first half of that file
and found a few by just reading the code thoroughly enough.
Who knows what else is still in there...
I have no more time for continuing this task right now though.
I guess it would help to get another few pairs of eyes on this.
And yes, hunting for bugs in C-string handling code can actually
be fun. It's certainly a good exercise to keep the average
programmer's brain (like the one I have) in shape :)
I'm starting to think that using pure C strings for all this
parsing stuff might indeed not be a good idea if we don't
get the code reviewed and more solid. Using some abstraction
that does bounds-checking on paths and uris might induce a
performance penalty, but will likely be more safe.
Because, we don't want any of this to be triggered over the network
input path, neither on clients nor on servers. That would be a nightmare.
Right now, the question whether this code will be in 1.6 is still open.
Including it in 1.7 would give us more time to improve the situation.
</rant>
Dang, I wish I had more time to spend on this,
Stefan
>
> On Wed, Jan 21, 2009 at 15:59, Hyrum K. Wright
> <hyrum_wright_at_mail.utexas.edu> wrote:
> > I updated my trunk client this morning to the latest HEAD (r35365) and then
> > tried to use that version to update another working copy. The following is the
> > result:
> >
> > (gdb) r up
> > Reading symbols for shared libraries .. done
> > D build/transform-sql.py
> > A build/transform_sql.py
> >
> > Program received signal EXC_BAD_ACCESS, Could not access memory.
> > Reason: KERN_INVALID_ADDRESS at address: 0x69737265
> > 0x0014965a in canonicalize (type=type_uri, path=0x69737265 <Address 0x69737265
> > out of bounds>, pool=0x10b6818) at subversion/libsvn_subr/dirent_uri.c:191
> > /Users/Hyrum/dev/svn-trunk/subversion/libsvn_subr/dirent_uri.c:191:5791:beg:0x14965a
> > (gdb)
> > No source file named address.
> > (gdb) backtrace
> > #0 0x0014965a in canonicalize (type=type_uri, path=0x69737265 <Address
> > 0x69737265 out of bounds>, pool=0x10b6818) at
> > subversion/libsvn_subr/dirent_uri.c:191
> > #1 0x0014acaa in svn_uri_canonicalize (uri=0x69737265 <Address 0x69737265 out
> > of bounds>, pool=0x10b6818) at subversion/libsvn_subr/dirent_uri.c:1108
> > #2 0x0014950c in local_style (type=type_uri, path=0x69737265 <Address
> > 0x69737265 out of bounds>, pool=0x10b6818) at subversion/libsvn_subr/dirent_uri.c:92
> > #3 0x00149e0b in svn_uri_local_style (uri=0x69737265 <Address 0x69737265 out of
> > bounds>, pool=0x10b6818) at subversion/libsvn_subr/dirent_uri.c:630
> > #4 0x0015a1f7 in svn_path_local_style (path=0x69737265 <Address 0x69737265 out
> > of bounds>, pool=0x10b6818) at subversion/libsvn_subr/path.c:61
> > #5 0x0000fc78 in notify (baton=0x744a78, n=0x10cab18, pool=0x10b6818) at
> > subversion/svn/notify.c:143
> > #6 0x0008c373 in close_file (file_baton=0x10b6890, expected_hex_digest=0x9a7188
> > "1f55d0a0cfe2a7d443dd0eb08ad26229", pool=0x10b6818) at
> > subversion/libsvn_wc/update_editor.c:4093
> > #7 0x0005d5a3 in close_file (file_baton=0x10b6860, text_checksum=0x9a7188
> > "1f55d0a0cfe2a7d443dd0eb08ad26229", pool=0x10b6818) at
> > subversion/libsvn_wc/ambient_depth_filter_editor.c:450
> > #8 0x0013abc4 in close_file (file_baton=0x10b6858, text_checksum=0x9a7188
> > "1f55d0a0cfe2a7d443dd0eb08ad26229", pool=0x10b6818) at
> > subversion/libsvn_delta/cancel.c:229
> > #9 0x0010ca5f in end_element (userdata=0x105cf88, state=240, nspace=0x8102f0
> > "svn:", elt_name=0x8135b0 "open-file") at subversion/libsvn_ra_neon/fetch.c:2070
> > #10 0x0011d227 in wrapper_endelm_cb (baton=0x1071ad8, state=240, nspace=0x8102f0
> > "svn:", name=0x8135b0 "open-file") at subversion/libsvn_ra_neon/util.c:1067
> > #11 0x00400b27 in end_element ()
> > #12 0x005dbb1d in doContent ()
> > #13 0x005dc9de in contentProcessor ()
> > #14 0x005da631 in doProlog ()
> > #15 0x005db3b5 in prologProcessor ()
> > #16 0x005d3a16 in XML_ParseBuffer ()
> > #17 0x0040121c in ne_xml_parse ()
> > #18 0x0011d50d in cancellation_callback (userdata=0x1022720, block=0x108ea08
> > "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n<S:update-report xmlns:S=\"svn:\"
> > xmlns:V=\"http://subversion.tigris.org/xmlns/dav/\" xmlns:D=\"DAV:\"
> > send-all=\"true\">\n<S:target-revision rev=\"35365\"/>\n<S:open-direct"...,
> > len=8192) at subversion/libsvn_ra_neon/util.c:1148
> > #19 0x003ff900 in do_inflate ()
> > #20 0x003f3501 in ne_read_response_block ()
> > #21 0x003f39c4 in ne_discard_response ()
> > #22 0x003f4b2b in ne_request_dispatch ()
> > #23 0x0011dd99 in svn_ra_neon__request_dispatch (code_p=0x0, req=0x1071a58,
> > extra_headers=0x10225a0, body=0x0, okay_1=200, okay_2=0, pool=0x101f418) at
> > subversion/libsvn_ra_neon/util.c:1449
> > #24 0x0011d724 in parsed_request (req=0x1071a58, ras=0x7e7800, method=0x18f5ec
> > "REPORT", url=0x7e7c10 "/repos/svn/!svn/vcc/default", body=0x0,
> > body_file=0x105d648, set_parser=0, startelm_cb=0x10a513 <start_element>,
> > cdata_cb=0x10c3ef <cdata_handler>, endelm_cb=0x10c534 <end_element>,
> > baton=0x105cf88, extra_headers=0x10225a0, status_code=0x0, spool_response=0,
> > pool=0x101f418) at subversion/libsvn_ra_neon/util.c:1228
> > #25 0x0011d95c in svn_ra_neon__parsed_request (sess=0x7e7800, method=0x18f5ec
> > "REPORT", url=0x7e7c10 "/repos/svn/!svn/vcc/default", body=0x0,
> > body_file=0x105d648, set_parser=0, startelm_cb=0x10a513 <start_element>,
> > cdata_cb=0x10c3ef <cdata_handler>, endelm_cb=0x10c534 <end_element>,
> > baton=0x105cf88, extra_headers=0x10225a0, status_code=0x0, spool_response=0,
> > pool=0x101f418) at subversion/libsvn_ra_neon/util.c:1291
> > #26 0x0010d7cc in reporter_finish_report (report_baton=0x105cf88,
> > pool=0x101f418) at subversion/libsvn_ra_neon/fetch.c:2388
> > #27 0x000541a0 in svn_wc_crawl_revisions4 (path=0x744a70 "",
> > adm_access=0x101f5a8, reporter=0x1a27f0, report_baton=0x105cf88,
> > restore_files=1, depth=svn_depth_unknown, honor_depth_exclude=1,
> > depth_compatibility_trick=0, use_commit_times=0, notify_func=0xfbc5 <notify>,
> > notify_baton=0x744a78, traversal_info=0x101f458, pool=0x101f418) at
> > subversion/libsvn_wc/adm_crawler.c:711
> > #28 0x00051938 in svn_client__update_internal (result_rev=0xbffff488,
> > path=0x744a70 "", revision=0xbffff65c, depth=svn_depth_unknown,
> > depth_is_sticky=0, ignore_externals=0, allow_unver_obstructions=0,
> > timestamp_sleep=0xbffff48c, send_copyfrom_args=1, ctx=0x1011a38, pool=0x101f418)
> > at subversion/libsvn_client/update.c:254
> > #29 0x00051bce in svn_client_update3 (result_revs=0x0, paths=0x744a40,
> > revision=0xbffff65c, depth=svn_depth_unknown, depth_is_sticky=0,
> > ignore_externals=0, allow_unver_obstructions=0, ctx=0x1011a38, pool=0x1011418)
> > at subversion/libsvn_client/update.c:337
> > #30 0x000172aa in svn_cl__update (os=0x10115c0, baton=0xbffff648,
> > pool=0x1011418) at subversion/svn/update-cmd.c:84
> > #31 0x0000e47f in main (argc=2, argv=0xbffff814) at subversion/svn/main.c:2122
> > (gdb)
> >
> > ------------------------------------------------------
> > http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1041272
> >
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1041277
Received on 2009-01-21 23:06:14 CET