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

RE: svn commit: r1166555 - /subversion/trunk/subversion/libsvn_ra_svn/cyrus_auth.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Thu, 8 Sep 2011 10:37:59 +0200

> -----Original Message-----
> From: stsp_at_apache.org [mailto:stsp_at_apache.org]
> Sent: donderdag 8 september 2011 10:05
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1166555 -
> /subversion/trunk/subversion/libsvn_ra_svn/cyrus_auth.c
>
> Author: stsp
> Date: Thu Sep 8 08:05:00 2011
> New Revision: 1166555
>
> URL: http://svn.apache.org/viewvc?rev=1166555&view=rev
> Log:
> Fix a possible crash in ra_svn if SASL authentication is active.
>
> * subversion/libsvn_ra_svn/cyrus_auth.c
> (svn_ra_svn__do_cyrus_auth): Give the auth callbacks sufficient
> lifetime to survive until connection pool cleanup. CyrusSASL
> needs the callbacks in the cleanup handler of this pool.
>
> Found by: Dave Huang <khym_at_azeotrope.org>
>
> Modified:
> subversion/trunk/subversion/libsvn_ra_svn/cyrus_auth.c
>
> Modified: subversion/trunk/subversion/libsvn_ra_svn/cyrus_auth.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/c
> yrus_auth.c?rev=1166555&r1=1166554&r2=1166555&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_ra_svn/cyrus_auth.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_svn/cyrus_auth.c Thu Sep 8
> 08:05:00 2011
> @@ -738,9 +738,7 @@ svn_ra_svn__do_cyrus_auth(svn_ra_svn__se
> const char *mechstring = "", *last_err = "", *realmstring;
> const char *local_addrport = NULL, *remote_addrport = NULL;
> svn_boolean_t success;
> - /* Reserve space for 3 callbacks (for the username, password and the
> - array terminator). */
> - sasl_callback_t callbacks[3];
> + sasl_callback_t *callbacks;
> cred_baton_t cred_baton;
> int i;
>
> @@ -776,6 +774,10 @@ svn_ra_svn__do_cyrus_auth(svn_ra_svn__se
> cred_baton.realmstring = realmstring;
> cred_baton.pool = pool;
>
> + /* Reserve space for 3 callbacks (for the username, password and the
> + array terminator). */
> + callbacks = apr_palloc(sess->conn->pool, sizeof(*callbacks) * 3);
> +
> /* Initialize the callbacks array. */

This isn't going to help when the baton that is passed (by pointer) to the callbacks is also allocated on the stack.
(The baton should probably move to heap as well if this is the right fix)

The function seems to assume that this callback infrastructure isn't used after returning from svn_ra_svn__do_cyrus_auth(), which would make allocating on the stack safe.
Any idea why this worked for years in 1.6 but now starts failing?

        Bert
Received on 2011-09-08 10:38:31 CEST

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