FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-33191: Fix refleak in posix_spawn by zware · Pull Request #6315 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

bpo-33191: Fix refleak in posix_spawn - #6315

Closed
zware wants to merge 3 commits into
python:masterfrom
zware:fix_refleak
ConversationCommits (3)ChecksFiles changed
Closed

bpo-33191: Fix refleak in posix_spawn#6315
zware wants to merge 3 commits into
python:masterfrom
zware:fix_refleak

Conversation

zware commented Mar 30, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

zware added type-bug An unexpected behavior, bug, or error needs backport to 3.7 labels Mar 30, 2018
serhiy-storchaka self-requested a review March 31, 2018 07:39
serhiy-storchaka reviewed Mar 31, 2018
View reviewed changes
Comment thread Modules/posixmodule.c

if(!PySequence_Check(file_actions_obj) | !PySequence_Size(file_actions_obj)) {
PyErr_SetString(PyExc_TypeError,"Each file_action element must be a non empty sequence");
if (file_actions_obj == NULL) {

serhiy-storchaka Mar 31, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is it possible?

Comment thread Modules/posixmodule.c
if (file_actions_obj == NULL) {
goto exit;
}
file_actions_obj = PySequence_Fast(file_actions_obj, "Each file_action element must be a non-empty sequence");

serhiy-storchaka Mar 31, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Would be nice to wrap this long line.

Comment thread Modules/posixmodule.c

long open_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));
long open_fd = PyLong_AsLong(PySequence_Fast_GET_ITEM(file_actions_obj, 1));
if(PyErr_Occurred()) {

serhiy-storchaka Mar 31, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

While you are here, could you please make the source conforming PEP 7: Add spaces after "if" and "switch", wrap long lines?

Comment thread Modules/posixmodule.c
}

long open_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1));
long open_fd = PyLong_AsLong(PySequence_Fast_GET_ITEM(file_actions_obj, 1));

serhiy-storchaka Mar 31, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

PyLong_AsLong() can call arbitrary code. If file_actions is a list, the size of file_actions_obj can be changed after this, and the following PySequence_Fast_GET_ITEM() could read past the end of the list.

I would require file_actions to be a tuple and use PyArg_ParseTuple() for parsing it.

serhiy-storchaka commented Mar 31, 2018

Copy link
Copy Markdown
Member

I have added more comments in #5109 because it is hard to add comment to the code not changed in a PR.

zware commented Mar 31, 2018

Copy link
Copy Markdown
Member Author

@serhiy-storchaka Would you mind to take this over? I would like to see this fixed before 3.7.0 final, but don't know when I'll be able to get back to it.

pablogsal mentioned this pull request Apr 1, 2018
Closed

pablogsal commented Apr 1, 2018

Copy link
Copy Markdown
Member

I have opened #6331 trying to address al the issues at once.

gpshead commented Apr 1, 2018

Copy link
Copy Markdown
Member

#6332 should obsolete this

zware commented Apr 2, 2018

Copy link
Copy Markdown
Member Author

Superseded by #6332 and/or #6331.

zware closed this Apr 2, 2018
zware deleted the fix_refleak branch April 2, 2018 18:22
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

serhiy-storchaka serhiy-storchaka left review comments

Assignees

No one assigned

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL