[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[STINNER Victor]

STINNER Victor  added the comment:

PPC64 Fedora 3.x, PPC64LE Fedora 3.x, s390x Debian 3.x and s390x RHEL 3.x 
buildbots are back to green thanks to this fix.

Oh wow, so it was a glibc bug: that was unexpected :-( Hopefully, the 
workaround is tiny and cheap!

> I think that after passing tests on builtbots this issue can be closed.

Right. I closed the PR 7663 and I close this issue.

Thank you all for investigating this issue, thanks Serhiy and Xiang for the 
review, and thanks Pablo for the fix!

--
resolution:  -> fixed
stage: patch review -> 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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Thank you Pablo! Seems failures in other tests that look like a race condition 
actually are caused by the glibc bug. I think that after passing tests on 
builtbots this issue can be closed.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset cb970730e3ca2522e9b1700dcaf0a06b7e898db6 by Serhiy Storchaka 
(Pablo Galindo) in branch 'master':
bpo-33630: Fix using of freed memory in old versions of glicb for 
posix_spawn(). (GH-7685)
https://github.com/python/cpython/commit/cb970730e3ca2522e9b1700dcaf0a06b7e898db6


--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Martin Panter]

Martin Panter  added the comment:

I doubt it is a race condition. “Waitpid” only returns after the child has 
terminated, and the file should be created as the child starts, before it 
terminates. Surely it is just the buggy glibc opening the DBDBDB file, when the 
test case expects it to create outfile.

--
nosy: +martin.panter

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

I have updated PR7685 to use the temporary list as per Serhiy comments.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

As I commented on GitHub, it seems that the race in the test (problem covered 
in PR7663) disappears once the memory problem is fixed (PR33630). As is a race 
condition I am not sure that this affirmation is true, but running 3 test 
suites in parallel that executes test_posix 200 times does not raise any 
problem on the buildbot after applying the patch in PR33630

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[STINNER Victor]

STINNER Victor  added the comment:

"It is better to have a single branch and add a comment that explains
a workaround. It can be removed after platforms with buggy glibc
versions will grow out of use."

Recently, I proposed to drop support for old Linux kernels, but some
people still use them and the code to support these kernels don't need
a lot of maintenance. So we keep the code :-)
https://mail.python.org/pipermail/python-dev/2018-January/151821.html

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

As I wrote on GitHub, I think that it is better to use the list approach always 
for two reasons:

1) Additional #if's and duplications clutter the code.

2) It is hard to test the alternate branch if the other branch is used on most 
developer computers.

It is better to have a single branch and add a comment that explains a 
workaround. It can be removed after platforms with buggy glibc versions will 
grow out of use.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Yes, as I wrote on GitHub.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Serhiy commented in the PR:

>__GLIBC__ and __GLIBC_MINOR__ give you the version of glibc used at >compile 
>time. But can not the different version be dynamically linked at >run time?

Should we use the list approach always to avoid problems with this?

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
pull_requests: +7298

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

I have updated the PR with a workaround.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[STINNER Victor]

STINNER Victor  added the comment:

> If we want to support older versions of glibc a temporary (and somewhat 
> inelegant) workaround is storing the temporaries in a list that the caller 
> passes and destroy the list after calling `posix_spawn`.

Creating a copy seems to be a reasonable tradeoff.

Another option is to detect the bug at runtime and raise an exception if the 
bug is detected. But I dislike this option: I hate when a function is available 
in the os module but calling it raises a NotImplementedError :-( Moreover, 
posix_spawn() seems to be a lot of benefit, so I would like to be able to use 
it on "old" glibc versions.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

If we want to support older versions of glibc a temporary (and somewhat 
inelegant) workaround is storing the temporaries in a list that the caller 
passes and destroy the list after calling `posix_spawn`.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Charalampos Stratakis]

Charalampos Stratakis  added the comment:

The latest stable Fedora's have glibc >= 2.26

Maybe the buildbot needs to be updated?

--
nosy: +cstratak

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Thank you Xiang! Thus this is a bug in old glibc. It was fixed in glibc 2.20: 
https://sourceware.org/bugzilla/show_bug.cgi?id=17048 .

What should we do with this? Add a workaround for the bug (this will complicate 
the code)? Or skip the test on glibc <2.20.

--
nosy: +alex

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Xiang Zhang]

Xiang Zhang  added the comment:

Serhiy, you need to check an early version, for example 2.17: 
https://github.com/bminor/glibc/blob/glibc-2.17/posix/spawn_faction_addopen.c

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I'm puzzled. It is not just documented that posix_spawn_file_actions_addopen() 
should make a copy, but the glibc implementation actually makes it: see for 
example 
https://github.com/bminor/glibc/blob/09533208febe923479261a27b7691abef297d604/posix/spawn_faction_addopen.c
 . I don't see a way of corrupting this memory.

Can non-glibc implementation be used here?

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Xiang Zhang]

Xiang Zhang  added the comment:

I checked glibc changelog and the copy behaviour is not implemented until 2014, 
glic 2.20. There is a CVE-2014-4043. My environment has glic 2.17 which seems 
still suffer the problem.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Xiang Zhang]

Xiang Zhang  added the comment:

I encounter the same failure when running test_posix suite with a debug build 
Python. 

[root@SZX1000328166 cpython]# ./python -m test test_posix
Run tests sequentially
0:00:00 load avg: 0.91 [1/1] test_posix
Warning -- files was modified by test_posix
  Before: []
  After:  
['\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcf0\udcd3B\udcb4\udc8b\x7f']
test test_posix failed -- multiple errors occurred; run in verbose mode for 
details
test_posix failed

== Tests result: FAILURE ==

1 test failed:
test_posix

Total duration: 459 ms
Tests result: FAILURE

Believe it or not, when I comment out the `Py_DECREF(path)`, the failure goes 
away.

--
nosy: +xiang.zhang

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Serhiy, I am checking and the only temporary Python object (the result of 
encoding/decoding) that is passed to C library function that I can find is in:


if (!PyArg_ParseTuple(file_action, "OiO"
 ";A open file_action tuple must have 5 elements",
 _obj, , PyUnicode_FSConverter, ,
 , ))
 {
 goto fail;
 }
 errno = posix_spawn_file_actions_addopen(file_actionsp,
 fd, PyBytes_AS_STRING(path), oflag, (mode_t)mode);
  Py_DECREF(path);  /* addopen copied it. */


And according to the manpage of posix_spawn_file_actions_addopen: "The string 
described by path shall be copied by the posix_spawn_file_actions_addopen() 
function.".
The object created by `PyArg_ParseTuple` (path) is only freed after calling 
`posix_spawn_file_actions_addopen` and in that function the contents are copied 
inside 
`file_actionsp`. So it should not be a problem.

In case there is still a problem, could you elaborate more about what object is 
being freed and how is being passed to a C library function?

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

For reference: posix_spawn() was implemented in issue20104.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Should we merge PR6693 and PR6725 before fixing the memory problem?

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

A separate PR is needed for the apparent reusing of freed memory in the 
implementation of posix_spawn. I am currently looking into it.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

In PR 7663 I am only fixing the race condition.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

The part that is related to \udcdb\udcdb\udcdb... is a problem in posix_spawn() 
implementation, not in tests. It can't be fixed by changing tests.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[STINNER Victor]

STINNER Victor  added the comment:

I'm not sure that the attached PR fixes all race conditions.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
pull_requests: +7278

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

The race condition is in tests (we can open a file before it be written in a 
child). The reed memory reusage is in the posix module. Didn't look at it 
close, but seems the content of a temporary Python object (the result of 
encoding/decoding) is passed to C library function, and this object is freed 
before the content be used.

--

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

@serhiy.storchaka Is the race condition/freed memory reusage in the test suite 
or in the posix module?

--
nosy: +pablogsal

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests:  -6734

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[twisteroid ambassador]

Change by twisteroid ambassador :


--
keywords: +patch
pull_requests: +6734
stage:  -> patch review

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Seems there are 2 or 3 different bugs here. Race condition, using freed memory, 
and maybe something other. I take this.

--
assignee:  -> serhiy.storchaka
type:  -> behavior

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

What is \udcdb\udcdb\udcdb...? Looks like a corrupted memory.

--
nosy: +serhiy.storchaka

___
Python tracker 

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

[issue33630] test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x

[STINNER Victor]

New submission from STINNER Victor :

http://buildbot.python.org/all/#/builders/53/builds/1062

(...)
test_close_file (test.test_posix.TestPosixSpawn) ... ok
test_dup2 (test.test_posix.TestPosixSpawn) ... ok
test_empty_file_actions (test.test_posix.TestPosixSpawn) ... ok
test_multiple_file_actions (test.test_posix.TestPosixSpawn) ... FAIL
test_no_such_executable (test.test_posix.TestPosixSpawn) ... ok
test_open_file (test.test_posix.TestPosixSpawn) ... ERROR
test_returns_pid (test.test_posix.TestPosixSpawn) ... ok
Warning -- files was modified by test_posix
  Before: []
  After:  
['\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb\udcdb']
 
test test_posix failed
test_specify_environment (test.test_posix.TestPosixSpawn) ... ok

==
ERROR: test_open_file (test.test_posix.TestPosixSpawn)
--
Traceback (most recent call last):
  File 
"/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/test/test_posix.py",
 line 1542, in test_open_file
with open(outfile) as f:
FileNotFoundError: [Errno 2] No such file or directory: '@test_42482_tmp'

==
FAIL: test_multiple_file_actions (test.test_posix.TestPosixSpawn)
--
Traceback (most recent call last):
  File 
"/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/test/test_posix.py",
 line 1488, in test_multiple_file_actions
self.assertEqual(os.waitpid(pid, 0), (pid, 0))
AssertionError: Tuples differ: (31319, 32512) != (31319, 0)

First differing element 1:
32512
0

- (31319, 32512)
? ^

+ (31319, 0)
? ^


--
Ran 101 tests in 2.175s

FAILED (failures=1, errors=1, skipped=9)
1 test failed again:
test_posix

--
components: Tests
messages: 317509
nosy: vstinner
priority: normal
severity: normal
status: open
title: test_posix: TestPosixSpawn fails on PPC64 Fedora 3.x
versions: Python 3.8

___
Python tracker 

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