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

Jump to bottom
Merged
merged 5 commits into from
Sep 21, 2018
Merged

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

merged 5 commits into from
Sep 21, 2018

Conversation

benjaminp
Copy link
Contributor
@benjaminp benjaminp commented Sep 13, 2018 •
edited by bedevere-bot
Loading

@benjaminp benjaminp force-pushed the benjamin-pickle-overflow branch from ea95daa to 59c974e Compare September 14, 2018 00:54
@benjaminp benjaminp changed the title bpo-1621: Avoid relying on signed overflow in _PyMemoTable_ResizeTable. bpo-34656: Avoid relying on signed overflow in _pickle memos. Sep 14, 2018
sir-sigurd
sir-sigurd reviewed Sep 14, 2018
View reviewed changes
Modules/_pickle.c Outdated
@@ -7061,7 +7057,7 @@ Unpickler_set_memo(UnpicklerObject *self, PyObject *obj)
error:
if (new_memo_size) {
i = new_memo_size;
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this line is redundant now.

serhiy-storchaka
serhiy-storchaka reviewed Sep 15, 2018
View reviewed changes
Modules/_pickle.c Outdated
Py_ssize_t to_process;

assert(min_size > 0);

/* Find the smallest valid table size >= min_size. */
while (new_size < min_size && new_size > 0)
while (new_size < min_size)
10000
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this cause an infinite loop if min_size > PY_SSIZE_T_MAX?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will fix.

sir-sigurd
sir-sigurd reviewed Sep 15, 2018
View reviewed changes
Modules/_pickle.c Outdated
@@ -909,7 +909,8 @@ 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))
size_t triple_used = self->mt_used * 3;
if (triple_used > self->mt_used && triple_used < self->mt_allocated * 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

triple_used > self->mt_used

Is this a wrapping/overflow detection?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

@benjaminp benjaminp force-pushed the benjamin-pickle-overflow branch from 7975314 to d8b389b Compare September 20, 2018 06:06
@benjaminp benjaminp merged commit a4ae828 into master Sep 21, 2018
@miss-islington
Copy link
Contributor

Thanks @benjaminp for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

@benjaminp benjaminp deleted the benjamin-pickle-overflow branch September 21, 2018 01:36
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 21, 2018
@benjaminp @miss-islington
closes bpo-34656: Avoid relying on signed overflow in _pickle memos. (p…
845165d 
…ythonGH-9261)

(cherry picked from commit a4ae828)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
@bedevere-bot
Copy link

GH-9465 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 21, 2018
@benjaminp @miss-islington
closes bpo-34656: Avoid relying on signed overflow in _pickle memos. (p…
fdc09c5 
…ythonGH-9261)

(cherry picked from commit a4ae828)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
@bedevere-bot
Copy link

GH-9466 is a backport of this pull request to the 3.6 branch.

miss-islington added a commit that referenced this pull request Sep 21, 2018
@miss-islington @benjaminp
closes bpo-34656: Avoid relying on signed overflow in _pickle memos. (G…
ef4306b 
…H-9261)

(cherry picked from commit a4ae828)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
miss-islington added a commit that referenced this pull request Sep 21, 2018
@miss-islington @benjaminp
closes bpo-34656: Avoid relying on signed overflow in _pickle memos. (G…
71a9c65 
…H-9261)

(cherry picked from commit a4ae828)

Co-authored-by: Benjamin Peterson <benjamin@python.org>
larryhastings pushed a commit that referenced this pull request Feb 25, 2019
@vstinner @larryhastings
[3.4] bpo-34656: Avoid relying on signed overflow in _pickle memos (G…
4b42d57 
…H-9261) (#11870)

* bpo-34656: Avoid relying on signed overflow in _pickle memos (GH-9261)

(cherry picked from commit a4ae828)
larryhastings pushed a commit that referenced this pull request Feb 26, 2019
@vstinner @larryhastings
closes bpo-34656: Avoid relying on signed overflow in _pickle memos. (G…
ef33dd6 
…H-9261) (#11869)

(cherry picked from commit a4ae828)
@httpsgithubcomxcainiao httpsgithubcomxcainiao mannequin mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sir-sigurd sir-sigurd sir-sigurd left review comments

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@benjaminp @miss-islington @bedevere-bot @sir-sigurd @serhiy-storchaka @the-knights-who-say-ni
0