On Thu, Jan 06, 2011 at 12:48:46AM +0100, Daniel Stenberg wrote:
> On Wed, 5 Jan 2011, Quinn Slack wrote:
>
> >This prevents my GnuTLS TLS-SRP patch from passing its tests, so
> >I'll try to figure this one out. I tried a bit to fix it, but the
> >obvious fix (freeing connect_sessionid if the
> >Curl_ssl_addsessionid call in gtls.c:704 fails) causes a segfault
> >that only occurs when *not* running in gdb. I'll keep trying, but
> >any help would be appreciated.
>
> I think it would be to free() the data in the wrong place since it
> isn't the function that allocates it. I would rather suggest a fix
> like my attachment shows, although I haven't actually tested it so
> I'll appreciate feedback!

Unfortunately, your patch introduces another problem: if
Curl_ssl_addsessionid fails in its second mem allocation call (the
Curl_clone_ssl_config one), it doesn't unset the passed-in sessionid ptr
from the session struct. Then Curl_ssl_close_all will examine the session
struct later and attempt to free that sessionid ptr, but gtls_connect_step3
(which malloc'd it) already freed it, and the program segfaults. This occurs
in at least tests 300-304 on GnuTLS.

The attached patch against git master (including your above-referenced patch
a9cd4f) changes Curl_ssl_addsessionid to clean up after itself properly
after a memory allocation failure. It passes torture tests 300-313 for me
with both OpenSSL and GnuTLS. Let me know if you'd like me to test it on
other configs.

-Quinn

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html