[mmcco@mykolab.com: Merge memleak fix from BoringSSL]

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[mmcco@mykolab.com: Merge memleak fix from BoringSSL]

Michael McConville-3
Does this look good?


----- Forwarded message from Michael McConville <[hidden email]> -----

Date: Sun, 20 Mar 2016 23:15:48 -0400
From: Michael McConville <[hidden email]>
To: [hidden email]
Subject: Merge memleak fix from BoringSSL

Looks like it applies to us:

https://boringssl.googlesource.com/boringssl/+/6b6e0b20893e2be0e68af605a60ffa2cbb0ffa64

Anecdotally, I need to check whether sk_X509_NAME_pop_free() are also
NULL-safe for us, or if BoringSSL made that change. One way or the
other, they removed their NULL checks. It's a little hard to confidently
discern because they're at least triple-nested macros, but I'll have the
time to spelunk eventually.


Index: src/ssl/s3_clnt.c
===================================================================
RCS file: /cvs/src/lib/libssl/src/ssl/s3_clnt.c,v
retrieving revision 1.137
diff -u -p -r1.137 s3_clnt.c
--- src/ssl/s3_clnt.c 11 Mar 2016 07:08:45 -0000 1.137
+++ src/ssl/s3_clnt.c 21 Mar 2016 03:08:40 -0000
@@ -1641,6 +1641,7 @@ ssl3_get_certificate_request(SSL *s)
     ERR_R_MALLOC_FAILURE);
  goto err;
  }
+ xn = NULL; /* avoid free in err block */
  }
 
  /* we should setup a certificate to return.... */
@@ -1658,6 +1659,7 @@ truncated:
     SSL_R_BAD_PACKET_LENGTH);
  }
 err:
+ X509_NAME_free(xn);
  if (ca_sk != NULL)
  sk_X509_NAME_pop_free(ca_sk, X509_NAME_free);
  return (ret);


----- End forwarded message -----

Reply | Threaded
Open this post in threaded view
|

Re: [mmcco@mykolab.com: Merge memleak fix from BoringSSL]

Todd C. Miller
On Sat, 26 Mar 2016 17:59:26 -0400, Michael McConville wrote:

> Does this look good?

OK millert@.  X509_NAME_free() (really asn1_item_combine_free)
just returns when passed a NULL pointer so this is safe.

 - todd