[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Emily Morehouse]

Emily Morehouse added the comment:

And thank you to Serhiy and Victor for the assistance and attention to this 
issue!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Emily Morehouse]

Emily Morehouse added the comment:

Resolving this since the fix and backports have been merged in.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Many thanks for you Victor for your wonderful tool.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[STINNER Victor]

STINNER Victor added the comment:

Ooookay, the bug is now fix on all branches. Thanks Eric & Emily for your PR 
and thanks Serhiy for the help ;-)

--
resolution:  -> fixed
stage: backport needed -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[STINNER Victor]

STINNER Victor added the comment:


New changeset eb3c52a0d273491e745e0cbff2b73900bb96aa45 by Victor Stinner (Emily 
Morehouse) in branch '3.5':
[3.5] bpo-30769: Fix reference leak introduced in 7770394 (GH-2416) (#2447)
https://github.com/python/cpython/commit/eb3c52a0d273491e745e0cbff2b73900bb96aa45


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Emily Morehouse]

Changes by Emily Morehouse :


--
pull_requests: +2503

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Just read the code. Find the function parse_envlist() in posixmodule.c and find 
similar code checking for '='.

Even if the leak is not detected by the test (but it is detected on Linux, 
maybe you use Windows?) it exists.

os.spawnve() also should be affected by this bug and fixed by the same patch, 
because it uses the same code.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Emily Morehouse]

Emily Morehouse added the comment:

I think I need a bit more direction for the 3.5 backport. The original test 
below passes:
./python -m test -R 3:3 -m test_execve_invalid_env test_os

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
stage: commit review -> backport needed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset 2d348f7a723db839aa18ce8213b8663ccb0a3d35 by Serhiy Storchaka 
(Emily Morehouse) in branch '3.6':
[3.6] bpo-30769: Fix reference leak introduced in 77703942c59 (GH-2416) (#2425)
https://github.com/python/cpython/commit/2d348f7a723db839aa18ce8213b8663ccb0a3d35


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The code in 3.5 differs from the code in master and 3.6, but it contains the 
same bug.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Emily Morehouse]

Changes by Emily Morehouse :


--
pull_requests: +2477

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[STINNER Victor]

STINNER Victor added the comment:


New changeset a7874c73c0c729bbec2fd4b077bd0eec276cfff4 by Victor Stinner (Eric 
N. Vander Weele) in branch 'master':
bpo-30769: Fix reference leak introduced in 77703942c59 (#2416)
https://github.com/python/cpython/commit/a7874c73c0c729bbec2fd4b077bd0eec276cfff4


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Emily Morehouse]

Changes by Emily Morehouse :


--
stage:  -> commit review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Eric N. Vander Weele]

Changes by Eric N. Vander Weele :


--
pull_requests: +2464

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Emily Morehouse]

Emily Morehouse added the comment:

I also found what Eric did, specifically lines 4913-4918 in the commit he 
mentioned introduced the bug:

if (PyBytes_GET_SIZE(key2) == 0 ||
strchr(PyBytes_AS_STRING(key2) + 1, '=') != NULL)
{
PyErr_SetString(PyExc_ValueError, "illegal environment variable name");
goto error;
}

One of the tricks is that the test that fails was added after the commit that 
introduced the bug.

--
nosy: +emilyemorehouse

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Eric N. Vander Weele]

Eric N. Vander Weele added the comment:

I think I may have found it.

$ git show 77703942c5997dff00c48f10df1b29b11645624c

Appears to indicate key2 and val2 are *not* decremented in the error 
conditions.  Should I PR a fix for this or let Serhiy resolve?

--
nosy: +ericvw

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Oh, my bad. Thank you for finding this leak Victor.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[STINNER Victor]

STINNER Victor added the comment:

Oops, wrong link to my email:
https://mail.python.org/pipermail/python-dev/2017-June/148489.html

I used this command:

 ./python bisect_test.py -R 3:3 test_os

Where the script comes from:
https://github.com/haypo/misc/blob/master/python/bisect_test.py

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[STINNER Victor]

STINNER Victor added the comment:

FYI I found this leak using my bisect_test.py script:
https://mail.python.org/pipermail/python-dev/2017-June/148368.html

Another hint: it seems like the leak was introduced recently ;-) Try "git log 
Modules/posixmodule.c".

Reminder: core developers, please don't fix the issue, but *explain how to fix 
it* ;-)

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30769] [EASY (C)] test_execve_invalid_env() of test_os leaks references

[STINNER Victor]

STINNER Victor added the comment:

The leak can reproduce only using this test:

def test_execve_invalid_env(self):
args = [sys.executable, '-c', 'pass']

# equal character in the enviroment variable name
newenv = os.environ.copy()
newenv["FRUIT=ORANGE"] = "lemon"
with self.assertRaises(ValueError):
os.execve(args[0], args, newenv)

It seems like the bug is in parse_envlist() function of Modules/posixmodule.c, 
when a key contains the '=' character.

--
keywords: +easy (C)
title: test_execve_invalid_env() of test_os leaks references -> [EASY (C)] 
test_execve_invalid_env() of test_os leaks references

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com