On Sun, Jun 08, 2008 at 09:49:35AM +0300, Daniel Shahaf wrote:
> 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.
Ooops, I didn't realise that the 1.4.x API was affected.
Thanks for pointing this out.
> 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).
Also, there's another crash bug already waiting for 1.5.1, namely r31223.
Which is about "Fix segfault when svn_ra_open3 is passed a bogus URL such
as 'bogusURL'." This affects 1.4 API users as well, because svn_ra_open2
calls svn_ra_open3, passing the URL parameter through.
r31223 was scheduled for 1.5.1 and not 1.5.0 by Eric because the crash
cannot happen with our svn(1) client. Because this is true also for the
(r31620, r31622) group, I opted for scheduling it for 1.5.1 as well.
Otherwise I would have nominated it for 1.5.0, because yes, crashing APIs
are really bad.
If we put either fix into 1.5.0, it does not make much sense to exclude
the other.
I guess Hyrum's not gonna be happy about an -rc10, but I would not object
putting those fixes into 1.5.0 if more people think it's necessary.
Stefan
- application/pgp-signature attachment: stored
Received on 2008-06-08 12:42:14 CEST