New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
os.set_inheritable() fails for O_PATH file descriptors on Linux #86946
Comments
Note: I filed this bug report after seeing rust-lang/rust#62425 and verifying that it was also reproducible on Python. Credit for discovering the underlying issue should go to Aleksa Sarai, and further discussion can be found there. # Background Linux has O_PATH file descriptors. These are file descriptors that refer to a specific path, without allowing any other kind of access to the file. They can't be used to read or write data; instead, they're intended to be used for use cases like the *at() functions. In that respect, they have similar semantics to O_SEARCH on other platforms (except that they also work on other file types, not just directories). More information on O_PATH file descriptors can be found in open(2) (https://www.man7.org/linux/man-pages/man2/open.2.html), or in the Rust PR linked above. # The problem As documented in the Rust PR linked above, *no* ioctl() calls will succeed on O_PATH file descriptors (they always fail with EBADF). Since os.set_inheritable() uses ioctl(FIOCLEX)/ioctl(FIONCLEX), it will fail on O_PATH file descriptors. This is easy to reproduce: >>> import os
>>> a = os.open("/", os.O_RDONLY)
>>> b = os.open("/", os.O_PATH)
>>> os.set_inheritable(a, True)
>>> os.set_inheritable(b, True) # Should succeed!
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OSError: [Errno 9] Bad file descriptor
>>> I believe this affects all versions of Python going back to version 3.4 (where os.set_inheritable()/os.get_inheritable() were introduced). # Possible fixes I see two potential paths for fixing this:
This is what Rust did. However, based on bpo-22258 I'm guessing there would be opposition to implementing this strategy in Python, on the grounds that the fcntl() route takes an extra syscall (which is fair).
This could be a very simple patch to Python/fileutils.c. I've attached a basic version of said patch (not sure if it matches standard coding conventions). Downsides: This would add 2 extra syscalls for O_PATH file descriptors, and 1 extra syscall for actual cases of invalid file descriptors (i.e. EBADF). However, I believe these are edge cases that shouldn't come up frequently. |
Doing two syscalls does not seem so bad. Linux may allow FIOCLEX on O_PATH in the future. |
I like this approach! |
Should I put together unit tests to go with the patch? Maybe |
I've put together some tests (patch attached). Should I PR this to python/cpython? |
Thanks cptpcrd for your bug report and your great fix! I like your tests ;-) |
Thanks cptpcrd for the bug report, the fix and for the two manual backports ;-) Note: Python 3.6 and 3.7 no longer accept bugfixes. |
No problem! I've noticed at least one other (relatively minor) issue in I haven't been keeping close track of 3.6/3.7's status, so I added them in without thinking it. Thanks for the reminder. |
Please do. But I suggest to open new issues.
Don't worry (be happy). Everybody is confused by the Versions field. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: