Hello,
Git can fail with a "malformed index nnn" error or cause a segmentation fault when executing the "git svn clone" command.
This is because the git-svn perl script calling into the external subversion library during fetch, wherein a bug exists that can corrupt the Perl Interpreter's stack maintained on the heap. The bug is in a custom swig typemap around the "result_digest" argument which is not handled in a safe manner: An absolute address on the perl stack is calculated beforehand to store the result of svn_swig_pl_from_md5(), but the function can trigger the stack to be reallocated elsewhere in memory (i.e. when trying to grow it but there is not enough space left) thus invaliding the address calculated earlier, causing memory corruption or segmentation fault.
This call is made once for each file retrieved from subversion. The fault is caused when the free available perl stack size is very small at the call, triggering a realloc() moving it elsewhere in memory. In my test environment --MSYS2 on windows 7--, this was caused when the available stack size left was 24 SV* slots. This means that a subversion repository with many file entries has a higher probability to fail while cloning than one with only a few file entries big.
Perl stack trace to the entry point of the offending code is:
SVN::TxDelta::apply(GLOB(0x60107ece8), GLOB(0x6011ee3a0), undef, SVN::Pool=REF(0x6011ee268)) called at /usr/share/perl5/site_perl/Git/SVN/Fetcher.pm line 368
Git::SVN::Fetcher::apply_textdelta(Git::SVN::Fetcher=HASH(0x6010236a0), HASH(0x6011ee2e0), undef, _p_apr_pool_t=SCALAR(0x60113e510)) called at /usr/lib/perl5/vendor_perl/SVN/Ra.pm line 623
SVN::Ra::Reporter::AUTOLOAD(SVN::Ra::Reporter=ARRAY(0x6010e9ee0), SVN::Pool=REF(0x6010ea228)) called at /usr/share/perl5/site_perl/Git/SVN/Ra.pm line 312
Git::SVN::Ra::gs_do_update(Git::SVN::Ra=HASH(0x60101f4f0), 384103, 384103, Git::SVN=HASH(0x60101fc58), Git::SVN::Fetcher=HASH(0x6010236a0)) called at /usr/share/perl5/site_perl/Git/SVN.pm line 1205
Git::SVN::do_fetch(Git::SVN=HASH(0x60101fc58), HASH(0x6010e9b20), 384103) called at /usr/share/perl5/site_perl/Git/SVN/Ra.pm line 475
Git::SVN::Ra::gs_fetch_loop_common(Git::SVN::Ra=HASH(0x60101f4f0), 384103, 384103, ARRAY(0x600788cc8), ARRAY(0x600788ce0)) called at /usr/share/perl5/site_perl/Git/SVN.pm line 179
Git::SVN::fetch_all("svn") called at /usr/lib/git-core/git-svn line 522
main::cmd_clone("http://...", "debug") called at /usr/lib/git-core/git-svn line 386
eval {...} called at /usr/lib/git-core/git-svn line 384The solution is to change the swig typemap in subversion to delay calculating the stack address where the result is stored after the call to the md5 function:
In subversion's subversion/subversion/bindings/swig/include/svn_types.swg, one can change
From
%typemap(argout) unsigned char *result_digest {
%append_output($1);
}To
%typemap(argout) unsigned char *result_digest {
SV* ucrdTemp = svn_swig_pl_from_md5($1);
%append_output(ucrdTemp);
}Swig will translate the above typemap in subversion/subversion/bindings/swig/perl/native/svn_delta.c
From
if (argvi >= items) EXTEND(sp,1); ST(argvi) = svn_swig_pl_from_md5(arg3); argvi++ ;
to
SV* ucrdTemp = svn_swig_pl_from_md5(arg3); if (argvi >= items) EXTEND(sp,1); ST(argvi) = ucrdTemp; argvi++ ;
(The stack address calculation is the results of calling the macro ST(argvi) ).
I am not a Perl expert, but ST(n) = any_function() seems to be a dangerous idiom because of the potential of any_function() to cause a Perl stack reallocation invaliding the address calculated beforehand by ST(n).
The above patch has been tested to work in my codebase both on windows 7 (with MSYS2) and on Arch Linux, where it would have caused a segmentation fault otherwise.
Fortunately, a patch has already been submitted to subversion with (github) revision a074af86c8764404b28ce99d0bedcb668a321408 (at https://github.com/apache/subversion/commit/a074af86c8764404b28ce99d0bedcb668a321408 ) on the trunk to handle this and a couple of other similar cases. But by the looks of it has not been picked up yet in the latest subversion 1.9.4 release or the 1.9.x branch, perhaps because this patch was identified in sanity checks rather than coming out from a perceivable production issue?
Although this issue is not in the Git source code, I thought reporting this error here first since it is Git that stresses the subversion perl bindings in such a level to cause the failure. Perhaps the patch can be applied first in those git binary releases that include subversion (I believe Git for windows is such a case) while in the mean time I submit this case for consideration in the subversion bug tracking system.
Yannis
***********************************************************************************
The Royal Bank of Scotland plc. Registered in Scotland No 90312.
Registered Office: 36 St Andrew Square, Edinburgh EH2 2YB.
Authorised by the Prudential Regulation Authority and regulated
by the Financial Conduct Authority and Prudential Regulation Authority.
The Royal Bank of Scotland N.V. is authorised and regulated by the
De Nederlandsche Bank and has its seat at Amsterdam, the
Netherlands, and is registered in the Commercial Register under
number 33002587. Registered Office: Gustav Mahlerlaan 350,
Amsterdam, The Netherlands. The Royal Bank of Scotland N.V. and
The Royal Bank of Scotland plc are authorised to act as agent for each
other in certain jurisdictions.
This e-mail message is confidential and for use by the addressee only.
If the message is received by anyone other than the addressee, please
return the message to the sender by replying to it and then delete the
message from your computer. Internet e-mails are not necessarily
secure. The Royal Bank of Scotland plc and The Royal Bank of Scotland
N.V. including its affiliates ("RBS group") does not accept responsibility
for changes made to this message after it was sent. For the protection
of RBS group and its clients and customers, and in compliance with
regulatory requirements, the contents of both incoming and outgoing
e-mail communications, which could include proprietary information and
Non-Public Personal Information, may be read by authorised persons
within RBS group other than the intended recipient(s). Whilst all reasonable care has been taken to avoid the transmission of viruses, it is the responsibility of the recipient to ensure that the onward transmission, opening or use of this message and any attachments will not adversely affect its systems or data. No responsibility is accepted by the RBS group in this regard and the recipient should carry out such virus and other checks as it considers appropriate.
Visit our website at www.rbs.com ***********************************************************************************
