classification
Title: ssl._ssl._test_decode_cert seems to leak memory with certain certificates in Python 2.6
Type: resource usage Stage: resolved
Components: Extension Modules Versions: Python 2.6
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, jcea, sYnfo
Priority: normal Keywords:

Created on 2013-09-03 11:46 by sYnfo, last changed 2013-10-20 19:53 by pitrou. This issue is now closed.

Messages (9)
msg196837 - (view) Author: Matěj Stuchlík (sYnfo) Date: 2013-09-03 11:46
Doing 'valgrind --suppressions=Misc/valgrind-python.supp ./python Lib/tests/test_ssl.py' I'm getting:

==322== LEAK SUMMARY:
==322==    definitely lost: 32 bytes in 1 blocks
==322==    indirectly lost: 392 bytes in 16 blocks
==322==      possibly lost: 1,617,191 bytes in 1,052 blocks
==322==    still reachable: 4,068,239 bytes in 3,201 blocks
==322==         suppressed: 0 bytes in 0 blocks


I managed to reduce that to a shorter reproducer:

"""
import os
import ssl

NULLBYTECERT = os.path.join(os.path.dirname(__file__) or os.curdir, "nullbytecert.pem")
p = ssl._ssl._test_decode_cert(NULLBYTECERT)
"""

where NULLBYTECERT is the cert introduced in issue18709.

Python 2.7 does not leak like this, and the PySSL_test_decode_certificate function looks the same there as in Python 2.6, so I assume it's something else that does the actual leaking.
msg196839 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2013-09-03 13:34
To ensure it's a real memory leak: do the figures increase when the code is called in a loop?
I would not consider a single-time malloc (stored in some static variable) to be a leak.
msg196842 - (view) Author: Matěj Stuchlík (sYnfo) Date: 2013-09-03 13:41
That's a good idea:

NULLBYTECERT = os.path.join(os.path.dirname(__file__) or os.curdir, "nullbytecert.pem")
for i in xrange(100):
    p = ssl._ssl._test_decode_cert(NULLBYTECERT)

gives

==1647== LEAK SUMMARY:
==1647==    definitely lost: 3,200 bytes in 100 blocks
==1647==    indirectly lost: 39,200 bytes in 1,600 blocks
==1647==      possibly lost: 591,285 bytes in 545 blocks
==1647==    still reachable: 1,955,652 bytes in 3,136 blocks
==1647==         suppressed: 0 bytes in 0 blocks

so yes, they do increase.
msg196845 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2013-09-03 13:59
Could you possibly check this in Python 2.7, 3.2 and 3.3?. Python 2.6 is open ONLY for security fixes, if any.
msg196846 - (view) Author: Matěj Stuchlík (sYnfo) Date: 2013-09-03 14:03
Ah, that is unfortunate. I did check it for 2.7 and 3.4, neither of those leak, I can check it for the rest tomorrow, but I imagine it'll be the same story.
msg196849 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2013-09-03 15:10
http://hg.python.org/cpython/rev/c4bbda2d4c49 looks relevant.
msg196850 - (view) Author: Matěj Stuchlík (sYnfo) Date: 2013-09-03 15:48
Potentially interesting part of the valgrind output:

==21685== 42,400 (3,200 direct, 39,200 indirect) bytes in 100 blocks are definitely lost in loss record 909 of 914
==21685==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==21685==    by 0x331B06315F: CRYPTO_malloc (in /usr/lib64/libcrypto.so.1.0.1e)
==21685==    by 0x331B0CD4EE: sk_new (in /usr/lib64/libcrypto.so.1.0.1e)
==21685==    by 0x331B0F2E42: ??? (in /usr/lib64/libcrypto.so.1.0.1e)
==21685==    by 0x331B0F2F7B: ??? (in /usr/lib64/libcrypto.so.1.0.1e)
==21685==    by 0x331B0F2884: ASN1_item_ex_d2i (in /usr/lib64/libcrypto.so.1.0.1e)
==21685==    by 0x331B0F3103: ASN1_item_d2i (in /usr/lib64/libcrypto.so.1.0.1e)
==21685==    by 0xB431892: _decode_certificate (_ssl.c:710)
==21685==    by 0xB431E57: PySSL_test_decode_certificate (_ssl.c:1025)
==21685==    by 0x49D187: PyEval_EvalFrameEx (ceval.c:3750)
==21685==    by 0x497A01: PyEval_EvalCodeEx (ceval.c:3000)
==21685==    by 0x497B41: PyEval_EvalCode (ceval.c:541)

_ssl.c:710 snippet:

(...)
 p = ext->value->data;
  if (method->it)
>     names = (GENERAL_NAMES*) (ASN1_item_d2i(NULL, &p, ext->value->length, ASN1_ITEM_ptr(method->it)));
  else
      names = (GENERAL_NAMES*) (method->d2i(NULL, &p, ext->value->length));
(...)
msg196851 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2013-09-03 16:07
Ah, http://hg.python.org/cpython/rev/80d491aaeed2/ as well then.
msg196852 - (view) Author: Matěj Stuchlík (sYnfo) Date: 2013-09-03 16:20
That seems to be it, no more leaking! Good job!
History
Date User Action Args
2013-10-20 19:53:31pitrousetstatus: open -> closed
resolution: out of date
stage: resolved
2013-09-03 16:20:17sYnfosetmessages: + msg196852
2013-09-03 16:07:27amaury.forgeotdarcsetmessages: + msg196851
2013-09-03 15:48:32sYnfosetmessages: + msg196850
2013-09-03 15:10:48amaury.forgeotdarcsetmessages: + msg196849
2013-09-03 14:03:14sYnfosetmessages: + msg196846
2013-09-03 13:59:27jceasetnosy: + jcea
messages: + msg196845
2013-09-03 13:41:48sYnfosetmessages: + msg196842
2013-09-03 13:34:02amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg196839
2013-09-03 11:46:41sYnfocreate