Skip to content

/cpython

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.

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

bpo-36367: Free buffer if realloc fails in tokenize.c #12442

Merged
merged 1 commit into from Mar 19, 2019
+8 −2

Conversation

Reviewers

@vstinner vstinner

@matrixise matrixise

@remilapeyre remilapeyre

Assignees
No one assigned
Labels
CLA signed skip news
Projects
None yet
Milestone
No milestone
7 participants
@pablogsal @remilapeyre @miss-islington @bedevere-bot @matrixise @vstinner @the-knights-who-say-ni
@pablogsal
Copy link
Member

pablogsal commented Mar 19, 2019 •
edited by bedevere-bot

https://bugs.python.org/issue36367

@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Mar 19, 2019

@bedevere-bot bedevere-bot added the awaiting merge label Mar 19, 2019

@pablogsal pablogsal added the skip news label Mar 19, 2019

@pablogsal pablogsal force-pushed the pablogsal:bpo36367 branch 2 times, most recently from 9c036e5 to dce2421 Mar 19, 2019

@matrixise

matrixise approved these changes Mar 19, 2019
View changes

Copy link
Contributor

matrixise left a comment

I had the same solution, seems to be fine for me.

@bedevere-bot bedevere-bot added awaiting core review and removed awaiting merge labels Mar 19, 2019

@remilapeyre

remilapeyre suggested changes Mar 19, 2019
View changes

Copy link
Contributor

remilapeyre left a comment

Hi @pablogsal I think there is the same issue in

cpython/Parser/tokenizer.c

Line 891 in dce2421

buf = (char *)PyMem_REALLOC(buf, newlen+1);
. Can you fix it too?

I think the cast to char * there is also not needed.
@remilapeyre

This comment has been minimized.

Sign in to view
Copy link
Contributor

remilapeyre commented Mar 19, 2019

As far as I can tell this issue is also present on

cpython/Parser/tokenizer.c

Line 963 in dce2421

newbuf = (char *)PyMem_REALLOC(newbuf,

@vstinner

vstinner reviewed Mar 19, 2019
View changes

Parser/tokenizer.c Outdated
@@ -618,7 +618,7 @@ static char *
translate_newlines(const char *s, int exec_input, struct tok_state *tok) {
int skip_next_lf = 0;
size_t needed_length = strlen(s) + 2, final_length;
char *buf, *current;
char *buf, *current, *result;

This comment has been minimized.

Sign in to view
Copy link
@vstinner

vstinner Mar 19, 2019

Member

Can you move result declaration where it's used? Python 3.8 now allows C99 ;-) (see PEP 7)

This comment has been minimized.

Sign in to view
Copy link
@pablogsal

pablogsal Mar 19, 2019

Author Member

Done! :)
@pablogsal

This comment has been minimized.

Sign in to view
Copy link
Member Author

pablogsal commented Mar 19, 2019

@remilapeyre I think for

cpython/Parser/tokenizer.c

Line 891 in dce2421

buf = (char *)PyMem_REALLOC(buf, newlen+1);
is not a problem as is handle correctly afterwards:

cpython/Parser/tokenizer.c

Lines 893 to 899 in dce2421

if (buf == NULL) {
PyMem_FREE(tok->buf);
tok->buf = NULL;
PyMem_FREE(newtok);
tok->done = E_NOMEM;
return EOF;
}

@pablogsal pablogsal force-pushed the pablogsal:bpo36367 branch from dce2421 to aeba989 Mar 19, 2019

@pablogsal pablogsal force-pushed the pablogsal:bpo36367 branch from aeba989 to 0650c4e Mar 19, 2019

@remilapeyre

remilapeyre approved these changes Mar 19, 2019
View changes

Copy link
Contributor

remilapeyre left a comment

@pablogsal Yes it is, I think you patch is correct now.

@pablogsal pablogsal merged commit cb90c89 into python:master Mar 19, 2019
5 checks passed

5 checks passed

Azure Pipelines PR #20190319.53 succeeded
Details
bedevere/issue-number Issue number 36367 found
Details
bedevere/news "skip news" label found
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@pablogsal pablogsal deleted the pablogsal:bpo36367 branch Mar 19, 2019

@bedevere-bot bedevere-bot removed the awaiting core review label Mar 19, 2019

@vstinner vstinner added needs backport to 2.7 needs backport to 3.7 labels Mar 20, 2019

@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Mar 20, 2019

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7.
🐍🍒⛏🤖
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Mar 20, 2019

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

vstinner added a commit to vstinner/cpython that referenced this pull request Mar 20, 2019

@pablogsal @vstinner
bpo-36367: Free buffer if realloc fails in tokenize.c (pythonGH-12442)
f0f6835

@bedevere-bot bedevere-bot removed the needs backport to 2.7 label Mar 20, 2019

@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Mar 20, 2019

GH-12470 is a backport of this pull request to the 2.7 branch.

vstinner added a commit to vstinner/cpython that referenced this pull request Mar 20, 2019

@pablogsal @vstinner
bpo-36367: Free buffer if realloc fails in tokenize.c (pythonGH-12442)
32e16c9

@bedevere-bot bedevere-bot removed the needs backport to 3.7 label Mar 20, 2019

@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Mar 20, 2019

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

vstinner added a commit that referenced this pull request Mar 20, 2019

@vstinner
bpo-36367: Free buffer if realloc fails in tokenize.c (GH-12442) (GH-… 
…12471)
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
65b9849

vstinner added a commit that referenced this pull request Mar 20, 2019

@vstinner
bpo-36367: Free buffer if realloc fails in tokenize.c (GH-12442) (GH-… 
…12470)

(cherry picked from commit cb90c89)
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
469b0a5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.