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 384
The 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
***********************************************************************************
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
