8000 bpo-34656: Avoid relying on signed overflow in _pickle memos. by benjaminp · Pull Request #9261 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-34656: Avoid relying on signed overflow in _pickle memos. #9261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 21, 2018
Merged

bpo-34656: Avoid relying on signed overflow in _pickle memos. #9261

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
abc4e5d
bpo-34656: Avoid relying on signed overflow in _pickle memos.
benjaminp Sep 13, 2018
c7a73fc
delete redundant line
benjaminp Sep 14, 2018
d8a3e61
do more arithmetic in size_t
benjaminp Sep 15, 2018
853afa2
add braces
benjaminp Sep 15, 2018
d8b389b
simplify & add a comment
benjaminp Sep 20, 2018
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
62 changes: 31 additions & 31 deletions Modules/_pickle.c
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -602,9 +602,9 @@ typedef struct {
} PyMemoEntry;

typedef struct {
Py_ssize_t mt_mask;
Py_ssize_t mt_used;
Py_ssize_t mt_allocated;
size_t mt_mask;
size_t mt_used;
size_t mt_allocated;
PyMemoEntry *mt_table;
} PyMemoTable;

Expand Down Expand Up @@ -650,8 +650,8 @@ typedef struct UnpicklerObject {
/* The unpickler memo is just an array of PyObject *s. Using a dict
is unnecessary, since the keys are contiguous ints. */
PyObject **memo;
Py_ssize_t memo_size; /* Capacity of the memo array */
Py_ssize_t memo_len; /* Number of objects in the memo */
size_t memo_size; /* Capacity of the memo array */
size_t memo_len; /* Number of objects in the memo */

PyObject *pers_func; /* persistent_load() method, can be NULL. */
PyObject *pers_func_self; /* borrowed reference to self if pers_func
Expand Down Expand Up @@ -737,7 +737,6 @@ PyMemoTable_New(void)
static PyMemoTable *
PyMemoTable_Copy(PyMemoTable *self)
{
Py_ssize_t i;
PyMemoTable *new = PyMemoTable_New();
if (new == NULL)
return NULL;
Expand All @@ -754,7 +753,7 @@ PyMemoTable_Copy(PyMemoTable *self)
PyErr_NoMemory();
return NULL;
}
for (i = 0; i < self->mt_allocated; i++) {
for (size_t i = 0; i < self->mt_allocated; i++) {
Py_XINCREF(self->mt_table[i].me_key);
}
memcpy(new->mt_table, self->mt_table,
Expand Down Expand Up @@ -800,7 +799,7 @@ _PyMemoTable_Lookup(PyMemoTable *self, PyObject *key)
{
size_t i;
size_t perturb;
size_t mask = (size_t)self->mt_mask;
size_t mask = self->mt_mask;
PyMemoEntry *table = self->mt_table;
PyMemoEntry *entry;
Py_hash_t hash = (Py_hash_t)key >> 3;
Expand All @@ -821,22 +820,24 @@ _PyMemoTable_Lookup(PyMemoTable *self, PyObject *key)

/* Returns -1 on failure, 0 on success. */
static int
_PyMemoTable_ResizeTable(PyMemoTable *self, Py_ssize_t min_size)
_PyMemoTable_ResizeTable(PyMemoTable *self, size_t min_size)
{
PyMemoEntry *oldtable = NULL;
PyMemoEntry *oldentry, *newentry;
Py_ssize_t new_size = MT_MINSIZE;
Py_ssize_t to_process;
size_t new_size = MT_MINSIZE;
size_t to_process;

assert(min_size > 0);

/* Find the smallest valid table size >= min_size. */
while (new_size < min_size && new_size > 0)
new_size <<= 1;
if (new_size <= 0) {
if (min_size > PY_SSIZE_T_MAX) {
PyErr_NoMemory();
return -1;
}

/* Find the smallest valid table size >= min_size. */
while (new_size < min_size) {
new_size <<= 1;
}
/* new_size needs to be a power of two. */
assert((new_size & (new_size - 1)) == 0);

Expand Down Expand Up @@ -909,10 +910,12 @@ PyMemoTable_Set(PyMemoTable *self, PyObject *key, Py_ssize_t value)
* Very large memo tables (over 50K items) use doubling instead.
* This may help applications with severe memory constraints.
*/
if (!(self->mt_used * 3 >= (self->mt_mask + 1) * 2))
if (SIZE_MAX / 3 >= self->mt_used && self->mt_used * 3 < self->mt_allocated * 2) {
return 0;
return _PyMemoTable_ResizeTable(self,
(self->mt_used > 50000 ? 2 : 4) * self->mt_used);
}
// self->mt_used is always < PY_SSIZE_T_MAX, so this can't overflow.
size_t desired_size = (self->mt_used > 50000 ? 2 : 4) * self->mt_used;
return _PyMemoTable_ResizeTable(self, desired_size);
}

#undef MT_MINSIZE
Expand Down Expand Up @@ -1376,9 +1379,9 @@ _Unpickler_Readline(UnpicklerObject *self, char **result)
/* Returns -1 (with an exception set) on failure, 0 on success. The memo array
will be modified in place. */
static int
_Unpickler_ResizeMemoList(UnpicklerObject *self, Py_ssize_t new_size)
_Unpickler_ResizeMemoList(UnpicklerObject *self, size_t new_size)
{
Py_ssize_t i;
size_t i;

assert(new_size > self->memo_size);

Expand All @@ -1397,9 +1400,9 @@ _Unpickler_ResizeMemoList(UnpicklerObject *self, Py_ssize_t new_size)

/* Returns NULL if idx is out of bounds. */
static PyObject *
_Unpickler_MemoGet(UnpicklerObject *self, Py_ssize_t idx)
_Unpickler_MemoGet(UnpicklerObject *self, size_t idx)
{
if (idx < 0 || idx >= self->memo_size)
if (idx >= self->memo_size)
return NULL;

return self->memo[idx];
Expand All @@ -1408,7 +1411,7 @@ _Unpickler_MemoGet(UnpicklerObject *self, Py_ssize_t idx)
/* Returns -1 (with an exception set) on failure, 0 on success.
This takes its own reference to `value`. */
static int
_Unpickler_MemoPut(UnpicklerObject *self, Py_ssize_t idx, PyObject *value)
_Unpickler_MemoPut(UnpicklerObject *self, size_t idx, PyObject *value)
{
PyObject *old_item;

Expand Down Expand Up @@ -4413,14 +4416,13 @@ static PyObject *
_pickle_PicklerMemoProxy_copy_impl(PicklerMemoProxyObject *self)
/*[clinic end generated code: output=bb83a919d29225ef input=b73043485ac30b36]*/
{
Py_ssize_t i;
PyMemoTable *memo;
PyObject *new_memo = PyDict_New();
if (new_memo == NULL)
return NULL;

memo = self->pickler->memo;
for (i = 0; i < memo->mt_allocated; ++i) {
for (size_t i = 0; i < memo->mt_allocated; ++i) {
PyMemoEntry entry = memo->mt_table[i];
if (entry.me_key != NULL) {
int status;
Expand Down Expand Up @@ -6843,7 +6845,7 @@ static PyObject *
_pickle_UnpicklerMemoProxy_copy_impl(UnpicklerMemoProxyObject *self)
/*[clinic end generated code: output=e12af7e9bc1e4c77 input=97769247ce032c1d]*/
{
Py_ssize_t i;
size_t i;
PyObject *new_memo = PyDict_New();
if (new_memo == NULL)
return NULL;
Expand Down Expand Up @@ -6994,8 +6996,7 @@ static int
Unpickler_set_memo(UnpicklerObject *self, PyObject *obj)
{
PyObject **new_memo;
Py_ssize_t new_memo_size = 0;
Py_ssize_t i;
size_t new_memo_size = 0;

if (obj == NULL) {
PyErr_SetString(PyExc_TypeError,
Expand All @@ -7012,7 +7013,7 @@ Unpickler_set_memo(UnpicklerObject *self, PyObject *obj)
if (new_memo == NULL)
return -1;

for (i = 0; i < new_memo_size; i++) {
for (size_t i = 0; i < new_memo_size; i++) {
Py_XINCREF(unpickler->memo[i]);
new_memo[i] = unpickler->memo[i];
}
Expand Down Expand Up @@ -7060,8 +7061,7 @@ Unpickler_set_memo(UnpicklerObject *self, PyObject *obj)

error:
if (new_memo_size) {
i = new_memo_size;
while (--i >= 0) {
for (size_t i = new_memo_size - 1; i != SIZE_MAX; i--) {
Py_XDECREF(new_memo[i]);
}
PyMem_FREE(new_memo);
Expand Down
0