1282731 - Fix various ssl_gtests leaks reported by LSan

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0002-Bug-1282731-Destroy-Poller-timers-on-gtest-shutdown.patch

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0003-Bug-1282731-Free-base-PRFileDesc-when-destroying-Dum.patch

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0004-Bug-1282731-Don-t-double-init-mp_ints-in-pqg_ParamGe.patch

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0005-Bug-1282731-Free-arena-in-tls13_HandleCertificateReq.patch

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0006-Bug-1282731-Don-t-leak-ss-ssl3.hs.srvVirtName.patch

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0007-Bug-1282731-Free-old-NPN-data-in-tls13_HandleEncrypt.patch

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0008-Bug-1282731-Free-sid-when-aborting-the-handshake-in-.patch

Comment 8

[Eric Rescorla (:ekr)]

Comment on attachment 8765821 [details] [diff] [review]
0002-Bug-1282731-Destroy-Poller-timers-on-gtest-shutdown.patch

Review of attachment 8765821 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 9

[Eric Rescorla (:ekr)]

Comment on attachment 8765823 [details] [diff] [review]
0003-Bug-1282731-Free-base-PRFileDesc-when-destroying-Dum.patch

Review of attachment 8765823 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 10

[Eric Rescorla (:ekr)]

Comment on attachment 8765838 [details] [diff] [review]
0007-Bug-1282731-Free-old-NPN-data-in-tls13_HandleEncrypt.patch

Review of attachment 8765838 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

::: lib/ssl/tls13con.c
@@ +2341,5 @@
>                              illegal_parameter);
>                  return SECFailure;
>              }
>          }
> +        SECITEM_FreeItem(&oldNpn, PR_FALSE);

Good catch.

Comment 11

[Eric Rescorla (:ekr)]

Comment on attachment 8765836 [details] [diff] [review]
0006-Bug-1282731-Don-t-leak-ss-ssl3.hs.srvVirtName.patch

Review of attachment 8765836 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 12

[Eric Rescorla (:ekr)]

Comment on attachment 8765833 [details] [diff] [review]
0005-Bug-1282731-Free-arena-in-tls13_HandleCertificateReq.patch

Review of attachment 8765833 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 13

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8765840 [details] [diff] [review]
0008-Bug-1282731-Free-sid-when-aborting-the-handshake-in-.patch

Review of attachment 8765840 [details] [diff] [review]:
-----------------------------------------------------------------

::: lib/ssl/ssl3con.c
@@ +9607,5 @@
>      (void)SSL3_SendAlert(ss, alert_fatal, desc);
>  /* FALLTHRU */
>  loser:
> +    if (sid && sid != ss->sec.ci.sid) {
> +        if (ss->sec.uncache)

please add braces

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/nss/rev/3a1bfe5e9ed9
https://hg.mozilla.org/projects/nss/rev/ea5513a4fa90
https://hg.mozilla.org/projects/nss/rev/9582c6dbf317
https://hg.mozilla.org/projects/nss/rev/6208863b9446
https://hg.mozilla.org/projects/nss/rev/52cacbeadbba
https://hg.mozilla.org/projects/nss/rev/d7185981d6e2
https://hg.mozilla.org/projects/nss/rev/6d5225e2f809