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

Re: you're going to kill me, but... [Re: svn commit: r31625 - branches/1.5.x]

From: Daniel Shahaf <d.s_at_daniel.shahaf.co.il>
Date: Sun, 8 Jun 2008 09:49:35 +0300 (Jerusalem Daylight Time)

David Glasser wrote on Sat, 7 Jun 2008 at 23:11 -0700:
> Um, I really don't want to drag this process out any longer. But as
> far as I can tell, without backporting these revisions, every single
> program written against the svn_repos API before 1.5.x (ie, the API
> which does not include svn_repos_set_repos_capabilities) which tries
> to commit anything to a repository with a start-commit hook will
> dereference NULL (ie, crash) every time.
>
> Please please please somebody convince me that I am wrong in that
> conclusion above. Because if we actually claim to care about API
> users, that's pretty serious.
>

The conclusion is correct.

The CLIENT_CAPABILITIES member of svn_repos_t is only written by the
new-in-1.5 svn_repos_remember_client_capabilities function, which isn't
called by any other svn_repos_* function.

However, svn_repos_fs_begin_txn_for_commit2 passes
$x=CLIENT_CAPABILITIES to svn_repos__hooks_start_commit(capabilities=$x),
which passes it to svn_cstring_join(strings=$x), which dereferences
STRINGS in the termination condition of the first for() loop. Since
STRINGS ultimately is CLIENT_CAPABILITIES, it is an uninitialised
pointer being dereferenced.

As Stefan suggested yesterday, another fix could be to initialise
client_capabilities to an empty array when creating an svn_repos_t
(and change the internal API in repos.h to disallow that array being
NULL).

Daniel

> --dave
>
> On Sat, Jun 7, 2008 at 11:08 PM, <glasser_at_tigris.org> wrote:
> > Author: glasser
> > Date: Sat Jun 7 23:08:11 2008
> > New Revision: 31625
> >
> > Log:
> > Vote for r31620/31622, and note that it may be important.
> >
> > Modified:
> > branches/1.5.x/STATUS
> >
> > Modified: branches/1.5.x/STATUS
> > URL: http://svn.collab.net/viewvc/svn/branches/1.5.x/STATUS?pathrev=31625&r1=31624&r2=31625
> > ==============================================================================
> > --- branches/1.5.x/STATUS Sat Jun 7 11:15:11 2008 (r31624)
> > +++ branches/1.5.x/STATUS Sat Jun 7 23:08:11 2008 (r31625)
> > @@ -110,10 +110,15 @@ Candidate changes for 1.5.1:
> > * r31620, r31622
> > Don't blindly assume that the client will always report capabilities.
> > Fixes a segfault in svn_repos__hooks_start_commit(). svn(1) won't
> > - trigger it, but other clients could. This can wait for 1.5.1 on the
> > - same grounds as r31223.
> > + trigger it, but other clients could.
> > + Notes:
> > + stsp: This can wait for 1.5.1 on the same grounds as r31223.
> > + glasser: Um, doesn't not applying this segfault *every single
> > + program* written against the pre-1.5 svn_repos API if
> > + the repository has a start-commit hook? As much as I
> > + hate to drag out this process...
> > Votes:
> > - +1: stsp, danielsh
> > + +1: stsp, danielsh, glasser
> >
> > Approved changes (all releases):
> > ================================
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
> > For additional commands, e-mail: svn-help_at_subversion.tigris.org
> >
> >
>
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-08 08:49:56 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.