-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
Open
Labels
3.15new features, bugs and security fixesnew features, bugs and security fixesstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
Bug report
Bug description:
(pid, fd) = pty.fork() # os.forkpty()
print('os.get_inheritable(fd))https://docs.python.org/3/library/os.html#os.forkpty
https://docs.python.org/3/library/pty.html#pty.fork
Say nothing about that.
Lib/pty.py:
def fork():
"""fork() -> (pid, master_fd)
Fork and make the child a session leader with a controlling terminal."""
try:
pid, fd = os.forkpty()
except (AttributeError, OSError):
pass
else:
if pid == CHILD:
try:
os.setsid()
except OSError:
# os.forkpty() already set us session leader
pass
return pid, fd
master_fd, slave_fd = openpty()
pid = os.fork()
if pid == CHILD:
os.close(master_fd)
os.login_tty(slave_fd)
else:
os.close(slave_fd)
# Parent and child process.
return pid, master_fdAlso uses openpty() which also does not set O_CLOEXEC
Meanwhile, there is a C function posix_openpt(int flags); which allows setting O_CLOEXEC.
Seems, fd-leak safe solution is to boilerplate all the actions by hand. i.e. same way as pty.fork() does, but also implement openpty() using posix_openpt() + ptsname() / ptsname_r().
Just setting O_CLOEXEC after openpty() or after calling C forkpty() is not enough because of race-condition and possible file descriptor leak.
So, CLOEXEC behavior should be documented and race-condition prevented
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Linked PRs
Metadata
Metadata
Assignees
Labels
3.15new features, bugs and security fixesnew features, bugs and security fixesstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error