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

2016-03-26 Thread 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



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

2016-03-26 Thread Michael McConville
Does this look good?


- Forwarded message from Michael McConville  -

Date: Sun, 20 Mar 2016 23:15:48 -0400
From: Michael McConville 
To: libre...@openbsd.org
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 -  1.137
+++ src/ssl/s3_clnt.c   21 Mar 2016 03:08:40 -
@@ -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 -