[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: Philip Martin <philip.martin_at_wandisco.com>
Date: Thu, 08 Sep 2011 11:20:00 +0100

"Bert Huijben" <bert_at_qqmail.nl> writes:

>> --- 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)

In practice it doesn't matter, see below. We should probably change it
or add a comment.

> 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?

The original bug report explained it. During pool cleanup we call the
SASL function sasl_dispose and that looks at the callback structs. The
stack struct has random values and if the id member makes it look like
the logging callback it gets invoked. Most callbacks won't get invoked
at that stage so only particular values will cause a problem. Also
since the auth callback won't get invoked it explains why the baton on
the stack works.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Received on 2011-09-08 12:21:02 CEST

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