[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[STINNER Victor]

STINNER Victor  added the comment:

> No problem! I've noticed at least one other (relatively minor) issue in `os`, 
> so I may be submitting further bug reports.

Please do. But I suggest to open new issues.

> 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.

Don't worry (be happy). Everybody is confused by the Versions field.

--

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[cptpcrd]

cptpcrd  added the comment:

No problem! I've noticed at least one other (relatively minor) issue in `os`, 
so I may be submitting further bug reports.

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.

--

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[STINNER Victor]

STINNER Victor  added the comment:

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.
https://devguide.python.org/#status-of-python-branches

--
versions:  -Python 3.6, Python 3.7

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 6893523bed4ad701838715380659c245eec71d48 by cptpcrd in branch 
'3.9':
bpo-42780: Fix set_inheritable() for O_PATH file descriptors on Linux 
(GH-24172) (GH-24278)
https://github.com/python/cpython/commit/6893523bed4ad701838715380659c245eec71d48


--

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 844ec0ba6606b60a59b7da82c54c1e646424259c by cptpcrd in branch 
'3.8':
bpo-42780: Fix set_inheritable() for O_PATH file descriptors on Linux 
(GH-24172) (GH-24277)
https://github.com/python/cpython/commit/844ec0ba6606b60a59b7da82c54c1e646424259c


--

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[cptpcrd]

Change by cptpcrd :


--
pull_requests: +23101
pull_request: https://github.com/python/cpython/pull/24278

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[cptpcrd]

Change by cptpcrd :


--
pull_requests: +23100
pull_request: https://github.com/python/cpython/pull/24277

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[STINNER Victor]

STINNER Victor  added the comment:

Thanks cptpcrd for your bug report and your great fix! I like your tests ;-)

--
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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[miss-islington]

Change by miss-islington :


--
pull_requests: +23093
pull_request: https://github.com/python/cpython/pull/24270

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 3.0 -> 4.0
pull_requests: +23092
pull_request: https://github.com/python/cpython/pull/24269

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 7dc71c425cf6aa6a4070a418dce5d95ca435c79f by cptpcrd in branch 
'master':
bpo-42780: Fix set_inheritable() for O_PATH file descriptors on Linux (GH-24172)
https://github.com/python/cpython/commit/7dc71c425cf6aa6a4070a418dce5d95ca435c79f


--

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[cptpcrd]

Change by cptpcrd :


--
pull_requests: +22999
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/24172

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[cptpcrd]

cptpcrd  added the comment:

I've put together some tests (patch attached). Should I PR this to 
python/cpython?

--
Added file: https://bugs.python.org/file49721/set-inheritable-test.patch

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[cptpcrd]

cptpcrd  added the comment:

> I like this approach!

Should I put together unit tests to go with the patch? Maybe 
`test_os.FDInheritanceTests.test_set_inheritable_o_path()`?

--

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[STINNER Victor]

STINNER Victor  added the comment:

> 2. On Linux, fall back on fcntl() if ioctl(FIOCLEX) fails with EBADF.

I like this approach!

--

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[Benjamin Peterson]

Benjamin Peterson  added the comment:

Doing two syscalls does not seem so bad.

Linux may allow FIOCLEX on O_PATH in the future.

--
nosy: +benjamin.peterson
versions: +Python 3.6, Python 3.7

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[Alexey Izbyshev]

Change by Alexey Izbyshev :


--
components: +Library (Lib)
nosy: +vstinner
versions:  -Python 3.6, Python 3.7

___
Python tracker 

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

[issue42780] os.set_inheritable() fails for O_PATH file descriptors on Linux

[cptpcrd]

New submission from cptpcrd :

Note: I filed this bug report after seeing 
https://github.com/rust-lang/rust/pull/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 "", line 1, in 
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:

1. Don't use ioctl(FIOCLEX) at all on Linux.

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).

2. On Linux, fall back on fcntl() if ioctl(FIOCLEX) fails with EBADF.

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.

--
files: set-inheritable-o-path.patch
keywords: patch
messages: 384016
nosy: cptpcrd
priority: normal
severity: normal
status: open
title: os.set_inheritable() fails for O_PATH file descriptors on Linux
type: behavior
versions: Python 3.10, Python 3.6, Python 3.7, Python 3.8, Python 3.9
Added file: https://bugs.python.org/file49706/set-inheritable-o-path.patch

___
Python tracker 

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