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

Re: svn+ssh crash on checkout

From: Hyrum K. Wright <hyrum_wright_at_mail.utexas.edu>
Date: Fri, 19 Dec 2008 07:44:43 -0600

Julian Foad wrote:
> On Fri, 2008-12-19 at 10:23 +0000, Julian Foad wrote:
>> On Fri, 2008-12-19 at 00:11 -0600, Hyrum K. Wright wrote:
>>> I just attempted to checkout a working copy via svn+ssh:
>>>
>>> hyrum-wrights-macbook-pro:dev Hyrum$ svnd co svn+ssh://example.com/var/svn/sdp out
>>> hwright_at_example.com's password:
>>> Bus error
>
> Please test this fix:
>
> [[[
> Fix a crash in Cyrus SASL auth support. From looking at the history it
> appears that the bug lay dormant until r33866 which started passing strings
> that are actually unmodifiable.
>
> Found by: hwright
>
> * subversion/libsvn_ra_svn/cyrus_auth.c
> (try_auth): Don't try to modify a constant string; allocate a new one.
> --This line, and those below, will be ignored--
>
> Index: subversion/libsvn_ra_svn/cyrus_auth.c
> ===================================================================
> --- subversion/libsvn_ra_svn/cyrus_auth.c (revision 34826)
> +++ subversion/libsvn_ra_svn/cyrus_auth.c (working copy)
> @@ -401,10 +401,12 @@ static svn_error_t *try_auth(svn_ra_svn_
> /* For anything else, delete the mech from the list
> and try again. */
> {
> - char *dst = strstr(mechstring, mech);
> - char *src = dst + strlen(mech);
> - while ((*dst++ = *src++) != '\0')
> - ;
> + const char *pmech = strstr(mechstring, mech);
> + const char *head = apr_pstrndup(pool, mechstring,
> + pmech - mechstring);
> + const char *tail = pmech + strlen(mech);
> +
> + mechstring = apr_pstrcat(pool, head, tail, NULL);
> again = TRUE;
> }
> }
> ]]]
>
> It adds two pool allocations per mechanism, but I think it's clearer and
> unless I'm much mistaken these auth's won't be performed huge numbers of
> times in a tight loop.

This does indeed fix the bug. Thanks!

> r33866 was:
> [[[
> ------------------------------------------------------------------------
> r33866 | glasser | 2008-10-23 18:53:32 +0100 (Thu, 23 Oct 2008) | 13 lines
>
> Fix regression in 1.5.x which made ra_svn clients compiled against Cyrus
> use ANONYMOUS even if EXTERNAL was available.
>
> * subversion/libsvn_ra_svn/cyrus_auth.c
> (svn_ra_svn__do_cyrus_auth): Prefer EXTERNAL to ANONYMOUS.
> [...]
> ]]]
>
> I haven't yet checked whether r33866 has been back-ported, and so
> whether this fix need to be back-ported.

Looking at CHANGES, it appears the r33866 has been backported for 1.5.5. We'd
better nominate the fix.

-Hyrum

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=987444

Received on 2008-12-19 14:45:09 CET

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