Skip to content

Commit fb0b942

Browse files
authored
gh-87138: convert blake2b/2s types to heap types (#127669)
1 parent 9ddc388 commit fb0b942

File tree

1 file changed

+69
-20
lines changed

1 file changed

+69
-20
lines changed

Modules/blake2module.c

Lines changed: 69 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -379,13 +379,13 @@ class _blake2.blake2s "Blake2Object *" "&PyBlake2_BLAKE2sType"
379379
static Blake2Object *
380380
new_Blake2Object(PyTypeObject *type)
381381
{
382-
Blake2Object *self;
383-
self = (Blake2Object *)type->tp_alloc(type, 0);
382+
Blake2Object *self = PyObject_GC_New(Blake2Object, type);
384383
if (self == NULL) {
385384
return NULL;
386385
}
387386
HASHLIB_INIT_MUTEX(self);
388387

388+
PyObject_GC_Track(self);
389389
return self;
390390
}
391391

@@ -454,7 +454,28 @@ py_blake2b_or_s_new(PyTypeObject *type, PyObject *data, int digest_size,
454454
}
455455

456456
self->impl = type_to_impl(type);
457-
457+
// Ensure that the states are NULL-initialized in case of an error.
458+
// See: py_blake2_clear() for more details.
459+
switch (self->impl) {
460+
#if HACL_CAN_COMPILE_SIMD256
461+
case Blake2b_256:
462+
self->blake2b_256_state = NULL;
463+
break;
464+
#endif
465+
#if HACL_CAN_COMPILE_SIMD128
466+
case Blake2s_128:
467+
self->blake2s_128_state = NULL;
468+
break;
469+
#endif
470+
case Blake2b:
471+
self->blake2b_state = NULL;
472+
break;
473+
case Blake2s:
474+
self->blake2s_state = NULL;
475+
break;
476+
default:
477+
Py_UNREACHABLE();
478+
}
458479
// Using Blake2b because we statically know that these are greater than the
459480
// Blake2s sizes -- this avoids a VLA.
460481
uint8_t salt_[HACL_HASH_BLAKE2B_SALT_BYTES] = { 0 };
@@ -595,7 +616,7 @@ py_blake2b_or_s_new(PyTypeObject *type, PyObject *data, int digest_size,
595616

596617
return (PyObject *)self;
597618
error:
598-
Py_XDECREF(self);
619+
Py_XDECREF(self);
599620
return NULL;
600621
}
601622

@@ -875,46 +896,70 @@ static PyGetSetDef py_blake2b_getsetters[] = {
875896
{NULL}
876897
};
877898

878-
879-
static void
880-
py_blake2b_dealloc(Blake2Object *self)
899+
static int
900+
py_blake2_clear(PyObject *op)
881901
{
902+
Blake2Object *self = (Blake2Object *)op;
903+
// The initialization function uses PyObject_GC_New() but explicitly
904+
// initializes the HACL* internal state to NULL before allocating
905+
// it. If an error occurs in the constructor, we should only free
906+
// states that were allocated (i.e. that are not NULL).
882907
switch (self->impl) {
883908
#if HACL_CAN_COMPILE_SIMD256
884909
case Blake2b_256:
885-
if (self->blake2b_256_state != NULL)
910+
if (self->blake2b_256_state != NULL) {
886911
Hacl_Hash_Blake2b_Simd256_free(self->blake2b_256_state);
912+
self->blake2b_256_state = NULL;
913+
}
887914
break;
888915
#endif
889916
#if HACL_CAN_COMPILE_SIMD128
890917
case Blake2s_128:
891-
if (self->blake2s_128_state != NULL)
918+
if (self->blake2s_128_state != NULL) {
892919
Hacl_Hash_Blake2s_Simd128_free(self->blake2s_128_state);
920+
self->blake2s_128_state = NULL;
921+
}
893922
break;
894923
#endif
895924
case Blake2b:
896-
// This happens if we hit "goto error" in the middle of the
897-
// initialization function. We leverage the fact that tp_alloc
898-
// guarantees that the contents of the object are NULL-initialized
899-
// (see documentation for PyType_GenericAlloc) to detect this case.
900-
if (self->blake2b_state != NULL)
925+
if (self->blake2b_state != NULL) {
901926
Hacl_Hash_Blake2b_free(self->blake2b_state);
927+
self->blake2b_state = NULL;
928+
}
902929
break;
903930
case Blake2s:
904-
if (self->blake2s_state != NULL)
931+
if (self->blake2s_state != NULL) {
905932
Hacl_Hash_Blake2s_free(self->blake2s_state);
933+
self->blake2s_state = NULL;
934+
}
906935
break;
907936
default:
908937
Py_UNREACHABLE();
909938
}
939+
return 0;
940+
}
910941

942+
static void
943+
py_blake2_dealloc(PyObject *self)
944+
{
911945
PyTypeObject *type = Py_TYPE(self);
912-
PyObject_Free(self);
946+
PyObject_GC_UnTrack(self);
947+
(void)py_blake2_clear(self);
948+
type->tp_free(self);
913949
Py_DECREF(type);
914950
}
915951

952+
static int
953+
py_blake2_traverse(PyObject *self, visitproc visit, void *arg)
954+
{
955+
Py_VISIT(Py_TYPE(self));
956+
return 0;
957+
}
958+
916959
static PyType_Slot blake2b_type_slots[] = {
917-
{Py_tp_dealloc, py_blake2b_dealloc},
960+
{Py_tp_clear, py_blake2_clear},
961+
{Py_tp_dealloc, py_blake2_dealloc},
962+
{Py_tp_traverse, py_blake2_traverse},
918963
{Py_tp_doc, (char *)py_blake2b_new__doc__},
919964
{Py_tp_methods, py_blake2b_methods},
920965
{Py_tp_getset, py_blake2b_getsetters},
@@ -923,7 +968,9 @@ static PyType_Slot blake2b_type_slots[] = {
923968
};
924969

925970
static PyType_Slot blake2s_type_slots[] = {
926-
{Py_tp_dealloc, py_blake2b_dealloc},
971+
{Py_tp_clear, py_blake2_clear},
972+
{Py_tp_dealloc, py_blake2_dealloc},
973+
{Py_tp_traverse, py_blake2_traverse},
927974
{Py_tp_doc, (char *)py_blake2s_new__doc__},
928975
{Py_tp_methods, py_blake2b_methods},
929976
{Py_tp_getset, py_blake2b_getsetters},
@@ -936,13 +983,15 @@ static PyType_Slot blake2s_type_slots[] = {
936983
static PyType_Spec blake2b_type_spec = {
937984
.name = "_blake2.blake2b",
938985
.basicsize = sizeof(Blake2Object),
939-
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE,
986+
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE
987+
| Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HEAPTYPE,
940988
.slots = blake2b_type_slots
941989
};
942990

943991
static PyType_Spec blake2s_type_spec = {
944992
.name = "_blake2.blake2s",
945993
.basicsize = sizeof(Blake2Object),
946-
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE,
994+
.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE
995+
| Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_HEAPTYPE,
947996
.slots = blake2s_type_slots
948997
};

0 commit comments

Comments
 (0)