Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions Lib/test/test_hashlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import tempfile
import threading
import unittest
import subprocess
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put the imports alphabetically sorted.

import textwrap
from test import support
from test.support import _4G, bigmemtest
from test.support import hashlib_helper
Expand Down Expand Up @@ -1201,6 +1203,43 @@ def test_readonly_types(self):
with self.assertRaisesRegex(TypeError, "immutable type"):
hash_type.value = False

@unittest.skipUnless(HASH is not None, 'need _hashlib')
def test_hashlib_init_memory_error_no_df(self):
"""gh-145301 regression test."""

try:
import _testcapi
if not hasattr(_testcapi, 'set_nomemory'):
self.skipTest('requires _testcapi.set_nomemory')
except ImportError:
self.skipTest('requires _testcapi')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use this, use import_helper for that.


code = textwrap.dedent("""
import sys
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sys is no more necessary

import _testcapi

if '_hashlib' in sys.modules:
del sys.modules['_hashlib']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessary.


_testcapi.set_nomemory(40, 41)
try:
import _hashlib
except (MemoryError, ImportError):
pass
finally:
_testcapi.remove_mem_hooks()
""")

rc = subprocess.call(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use assert_python_ok instead.

[sys.executable, '-c', code],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
# rc < 0 means crash (signal on Unix), which indicates double-free
# rc >= 0 means normal exit (even with MemoryError), which is expected
self.assertGreaterEqual(rc, 0,
"Process crashed - Loss double-free in _hashlib")


class KDFTests(unittest.TestCase):

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a crash when :mod:`hashlib` or :mod:`hmac` C extension module initialization fails.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you create two NEWS entries, one for hashlib and for hmac please with the same sentence:

:mod:`<name>`: fix a crash when the initialization of the underlying C extension module fails.

2 changes: 1 addition & 1 deletion Modules/_hashopenssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ py_hashentry_table_new(void) {

if (h->py_alias != NULL) {
if (_Py_hashtable_set(ht, (const void*)entry->py_alias, (void*)entry) < 0) {
PyMem_Free(entry);
/* entry is already in ht, will be freed by _Py_hashtable_destroy() */
goto error;
}
entry->refcnt++;
Expand Down
6 changes: 5 additions & 1 deletion Modules/hmacmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1457,7 +1457,11 @@ py_hmac_hinfo_ht_new(void)
do { \
int rc = py_hmac_hinfo_ht_add(table, KEY, value); \
if (rc < 0) { \
PyMem_Free(value); \
/* entry may already be in ht, will be freed by \
_Py_hashtable_destroy() */ \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just put re-align the \ directly and put the comment on one line (you can remove the "will be freed by _Py_hashtable_destroy() and mention that it will be freed "upon exit").

if (value->refcnt == 0) { \
PyMem_Free(value); \
} \
goto error; \
} \
else if (rc == 1) { \
Expand Down
Loading