Issue 8154: os.execlp('true') crashes the interpreter on 2.x (original) (raw)

Issue8154

Created on 2010-03-16 12:05 by doko, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
p.diff doko,2010-03-16 12:05
Messages (24)
msg101162 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2010-03-16 12:05
calling os.execlp('true') with the wrong number of arguments (missing 2nd arg), the interpreter crashes. fixed in 3.x, this is a backport of the patch to 2.x
msg101189 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2010-03-16 20:49
Looks ok, but I think it needs a test, and it looks like execve suffers from the same problem.
msg101195 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-03-16 22:34
As far as I can tell, it does not *crash* the interpreter. Instead, it replaces the python interpreter process with a "true" utility. $ ./python.exe Python 2.7a4+ (trunk:78816M, Mar 9 2010, 18:57:13) [GCC 4.2.1 (Apple Inc. build 5646) (dot 1)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> import os; os.execlp('true') $ echo ?0? 0 ?0 ./python.exe Python 2.7a4+ (trunk:78816M, Mar 9 2010, 18:57:13) [GCC 4.2.1 (Apple Inc. build 5646) (dot 1)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> import os; os.execlp('false') air:trunk sasha$ echo $? 1
msg101196 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-03-16 22:35
I forgot to add that this behavior is documented: os.execlp = execlp(file, *args) execlp(file, *args) Execute the executable file (which is searched for along $PATH) with argument list args, replacing the current process.
msg101199 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-03-16 23:19
The original report, has a better problem description: "In a windows debug build, an assertion is triggered when os.execvpe is called with an empty argument list: self.assertRaises(OSError, os.execvpe, 'no such app-', [], None)" The patch, /os.diff is complete with a test and should be easy to apply to the trunk. I would be -0 on the change. On one hand, POSIX seems to require a non-empty argument list, on the other hand at least Linux and MacOS X appear to be happy with os.execlp('true'). Furthemore, the proposed change will change the exception from os.execlp('no such program') from OSError to ValueError, which is not a backward compatible change. As such it is certainly not appropriate for bug-fix releases. As far as 3x series are concerned, I think ValueError exception should be documented.
msg101201 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-03-16 23:24
Adding Thomas. Thomas, Do you remember why your patch for was not backported?
msg101203 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2010-03-16 23:46
it does crash: $ python Python 2.6.5rc2 (r265rc2:78822, Mar 11 2010, 13:01:50) [GCC 4.4.3] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> os.execlp('true') Segmentation fault (core dumped) arg[0] (the command name) must be part of the second parameter. side notice: the execlpe looks inconsistent (both 2.x and 3.x). >>> os.execlpe('true') Traceback (most recent call last): File "", line 1, in File "/usr/lib/python2.6/os.py", line 335, in execlpe env = args[-1] IndexError: tuple index out of range shouldn't this be a ValueError as well?
msg101204 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-03-17 00:02
On Tue, Mar 16, 2010 at 7:46 PM, Matthias Klose <report@bugs.python.org> wrote: .. > it does crash: Strange. Can you report your Linux version and the stack trace from the crash? On Ubuntu, 2.6.24-27-generic #1 SMP Wed Jan 27 23:54:28 UTC 2010 i686 GNU/Linux, I see no crash. Moreover, I've just checked that the following C program works just fine: #include <unistd.h> int main() { execlp("ls", NULL); }
msg101208 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-03-17 00:34
It does not crash on Gentoo, either: rdmurray@maestro:~>uname -a Linux maestro 2.6.31-gentoo-r3 #1 SMP PREEMPT Thu Oct 22 20:13:19 EDT 2009 i686 Intel(R) Core(TM)2 CPU 6600 @ 2.40GHz GenuineIntel GNU/Linux rdmurray@maestro:~/python/release26-maint>./python Python 2.6.5rc2 (release26-maint:79008, Mar 16 2010, 20:29:24) [GCC 4.4.3] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> import os [36856 refs] >>> os.execlp('false') rdmurray@maestro:~/python/release26-maint>echo $? 1
msg101209 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2010-03-17 00:40
crash seen on both Debian unstable and recent Ubuntu lucid. (gdb) run Starting program: /home/doko/a.out process 30155 is executing new program: /bin/ls [Thread debugging using libthread_db enabled] Program received signal SIGSEGV, Segmentation fault. strrchr () at ../sysdeps/i386/strrchr.S:178 178 ../sysdeps/i386/strrchr.S: No such file or directory. in ../sysdeps/i386/strrchr.S (gdb) bt #0 strrchr () at ../sysdeps/i386/strrchr.S:178 #1 0x080524d2 in ?? () #2 0x0804fb2c in ?? () #3 0x00170bd6 in __libc_start_main (main=0x804fb10, argc=0, ubp_av=0xbffff534, init=0x805bd60, fini=0x805bd50, rtld_fini=0x11e0b0 <_dl_fini>, stack_end=0xbffff52c) at libc-start.c:226 #4 0x08049cf1 in ?? () stack trace from lucid; glibc-2.11.1, linux 2.6.32
msg101270 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-03-18 18:34
Crash reproduced under Mandriva Linux 2010.0 (glibc-2.10.1-6.2mnb2). Stack trace is as follows: (gdb) bt #0 0x00007fb59f2eba9a in strrchr () from /lib64/libc.so.6 #1 0x00000000004010ae in ?? () #2 0x0000000000400ff3 in ?? () #3 0x00007fb59f29091d in __libc_start_main () from /lib64/libc.so.6 #4 0x0000000000400d79 in ?? () #5 0x00007fff00162fa8 in ?? () #6 0x000000000000001c in ?? () #7 0x0000000000000000 in ?? () Also, please add the missing test when backporting the patch.
msg101271 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2010-03-18 18:47
> Thomas, > > Do you remember why your patch for was not backported? Probably an oversight only.
msg101274 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-03-18 19:32
I agree that this should be fixed, since we presumably want to be "strictly conforming" to the posix standards, but it looks like this is a regression in either linux or glibc. From the standard's rational section: Early proposals required that the value of argc passed to main() be "one or greater". This was driven by the same requirement in drafts of the ISO C standard. In fact, historical implementations have passed a value of zero when no arguments are supplied to the caller of the exec functions. This requirement was removed from the ISO C standard and subsequently removed from this volume of IEEE Std 1003.1-2001 as well. The wording, in particular the use of the word should, requires a Strictly Conforming POSIX Application to pass at least one argument to the exec function, thus guaranteeing that argc be one or greater when invoked by such an application. In fact, this is good practice, since many existing applications reference argv[0] without first checking the value of argc.
msg101278 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-03-18 20:18
On Thu, Mar 18, 2010 at 3:32 PM, R. David Murray <report@bugs.python.org> wrote: .. > I agree that this should be fixed, since we presumably want to be "strictly conforming" to the posix standards, > but it looks like this is a regression in either linux or glibc.  From the standard's rational section: > Let me add just make a few observations without drawing a conclusion: 1. This is not a crash of python. As far as I can tell, it is the executed program that crashes attempting to dereference argv[0] without checking argc first. On the platforms that support execution with argc=0, this is a bug in the application or in the C library. 2. Currently 3.x and 2.x python throw different exceptions from os.execlp('xyz') if xyz does not exist. Maybe os.execlp(one_arg) should raise OSError instead of ValueError. 3. Another alternative would be to change the signature from os.execlp(path, ...) to os.execlp(path, name, ...) and make os.execlp() raise TypeError if name is not supplied. This is an attractive possibility because it can be done by a trivial change in os.py while preserving the ability to exec with argc=0 for those who know what they are doing.
msg101329 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2010-03-19 14:46
commited to the trunk, commit to the 2.6 branch pending
msg101354 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2010-03-20 02:19
committed to the 2.6 branch as well
msg101373 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-03-20 14:19
I believe that backporting this change to 2.6 is inappropriate. It will more than likely cause perfectly correct code to stop working, and that is not something we like to do in a maintenance release. I believe that the bug on the debian/ubuntu side is actually that /bin/true crashes if argc is 0. Someone with a debian or ubuntu system should test this. In fact, reviewing this issue again after Alexander's post to python-dev, I don't think that this fix in its current form is advisable at all. It makes Python's execv function less flexible than the underlying system API, and more importantly changes its behavior in a backwards incompatible way. Python 3 is a different story in this regard, since the change went in before 3.1, and as noted it makes Python "strictly compliant" with the posix standard, which seems like an acceptable reason to decrease flexibility. Since it does trigger a crash on the windows equivalent API, the check should be conditional on platform. And it should generate a py3k warning on other platforms. I'm open to an argument that the API can be changed in 2.7, although I don't think we normally do that without a deprecation first. But I think there is no question that this change in its current form needs to be backed out of 2.6.
msg101432 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2010-03-21 17:20
> this change in its current form needs to be backed out of 2.6 done. I'll check for uses of execlp and execlpe. how should the divergency of execlp (raises ValueError), and execlpe (raises IndexError) be handled?
msg101434 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-03-21 17:27
Hello Could you please write the revision number when you speak about a commit? Text like “fixed in r4253” will become an helpful link. Thanks
msg101436 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2010-03-21 17:38
reverted in r79190 on the 2.6 branch
msg101438 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-03-21 18:20
Please see related .
msg103606 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-04-19 14:30
If this new feature stays in 3.x, shouldn't 2.7 have a -3 warning? Also, I would consider adding os.execlp(path) -> os.execlp(path, os.path.basename(path)) transformation to 2to3.
msg103607 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-04-19 14:47
I noticed that the change is still present in 2.7a. For what it's worth, I agree with David: """ Since it does trigger a crash on the windows equivalent API, the check should be conditional on platform. And it should generate a py3k warning on other platforms. """ I am changing classification to "feature request" and adding "Windows" to the list of components because if I understand David correctly, the title of the issue is valid only on Windows platform. (On other platforms os.execlp(path) may cause a crash of the program found at path, but not a crash of python interpreter itself.) I also remove 2.6 from the list of versions since after r79190 no further action is contemplated for 2.6 branch.
msg124226 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-17 16:21
OK, this went in to 2.7 without the OS conditional, and there has been no great hue and cry, so I guess it was safe enough :) As for the difference in error message between execlp and execlpe, I think that's fine. The execlpe index error message is accurate: execlpe requires an 'env' arg, and it was missing.
History
Date User Action Args
2022-04-11 14:56:58 admin set github: 52401
2010-12-17 16:21:09 r.david.murray set status: open -> closednosy:theller, doko, pitrou, paulsmith, benjamin.peterson, eric.araujo, r.david.murray, Alexander.Belopolskymessages: + resolution: acceptedstage: needs patch -> resolved
2010-04-19 14:47:22 Alexander.Belopolsky set versions: - Python 2.6nosy:theller, doko, pitrou, paulsmith, benjamin.peterson, eric.araujo, r.david.murray, Alexander.Belopolskymessages: + components: + Windowstype: crash -> enhancement
2010-04-19 14:30:07 Alexander.Belopolsky set messages: +
2010-04-04 15:58:50 paulsmith set nosy: + paulsmith
2010-03-21 18:20:52 Alexander.Belopolsky set messages: +
2010-03-21 17:38:30 doko set messages: +
2010-03-21 17:27:01 eric.araujo set nosy: + eric.araujomessages: +
2010-03-21 17:20:41 doko set messages: +
2010-03-20 14:19:42 r.david.murray set status: closed -> openresolution: fixed -> (no value)messages: + stage: patch review -> needs patch
2010-03-20 02:19:13 doko set status: open -> closedresolution: fixedmessages: +
2010-03-19 14:46:01 doko set messages: +
2010-03-18 20🔞59 Alexander.Belopolsky set messages: +
2010-03-18 19:32:08 r.david.murray set messages: +
2010-03-18 18:47:42 theller set messages: +
2010-03-18 18:34:40 pitrou set priority: hightype: crashversions: - Python 2.5nosy: + pitroumessages: + stage: patch review
2010-03-17 00:40:04 doko set messages: +
2010-03-17 00:34:45 r.david.murray set nosy: + r.david.murraymessages: +
2010-03-17 00:02:52 Alexander.Belopolsky set messages: +
2010-03-16 23:46:40 doko set messages: +
2010-03-16 23:24:36 Alexander.Belopolsky set nosy: + thellermessages: +
2010-03-16 23:19:02 Alexander.Belopolsky set messages: +
2010-03-16 22:35:26 Alexander.Belopolsky set messages: +
2010-03-16 22:34:19 Alexander.Belopolsky set nosy: + Alexander.Belopolskymessages: +
2010-03-16 20:49:29 benjamin.peterson set nosy: + benjamin.petersonmessages: +
2010-03-16 12:05:41 doko create