Skip to content

Commit 52c8efa

Browse files
authored
gh-145335: Fix os functions when passing fd -1 as path (#145439)
os.listdir(-1) and os.scandir(-1) now fail with OSError(errno.EBADF) rather than listing the current directory. os.listxattr(-1) now fails with OSError(errno.EBADF) rather than listing extended attributes of the current directory.
1 parent db41717 commit 52c8efa

File tree

4 files changed

+103
-32
lines changed

4 files changed

+103
-32
lines changed

Doc/library/os.rst

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2409,6 +2409,10 @@ features:
24092409
.. versionchanged:: 3.6
24102410
Accepts a :term:`path-like object`.
24112411

2412+
.. versionchanged:: next
2413+
``os.listdir(-1)`` now fails with ``OSError(errno.EBADF)`` rather than
2414+
listing the current directory.
2415+
24122416

24132417
.. function:: listdrives()
24142418

@@ -2939,6 +2943,10 @@ features:
29392943
.. versionchanged:: 3.7
29402944
Added support for :ref:`file descriptors <path_fd>` on Unix.
29412945

2946+
.. versionchanged:: next
2947+
``os.scandir(-1)`` now fails with ``OSError(errno.EBADF)`` rather than
2948+
listing the current directory.
2949+
29422950

29432951
.. class:: DirEntry
29442952

@@ -4574,6 +4582,10 @@ These functions are all available on Linux only.
45744582
.. versionchanged:: 3.6
45754583
Accepts a :term:`path-like object`.
45764584

4585+
.. versionchanged:: next
4586+
``os.listxattr(-1)`` now fails with ``OSError(errno.EBADF)`` rather than
4587+
listing extended attributes of the current directory.
4588+
45774589

45784590
.. function:: removexattr(path, attribute, *, follow_symlinks=True)
45794591

Lib/test/test_os/test_os.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2784,10 +2784,61 @@ def test_fpathconf_bad_fd(self):
27842784
'musl fpathconf ignores the file descriptor and returns a constant',
27852785
)
27862786
def test_pathconf_negative_fd_uses_fd_semantics(self):
2787+
if os.pathconf not in os.supports_fd:
2788+
self.skipTest('needs fpathconf()')
2789+
27872790
with self.assertRaises(OSError) as ctx:
27882791
os.pathconf(-1, 1)
27892792
self.assertEqual(ctx.exception.errno, errno.EBADF)
27902793

2794+
@support.subTests("fd", [-1, -5])
2795+
def test_negative_fd_ebadf(self, fd):
2796+
tests = [(os.stat, fd)]
2797+
if hasattr(os, "statx"):
2798+
tests.append((os.statx, fd, 0))
2799+
if os.chdir in os.supports_fd:
2800+
tests.append((os.chdir, fd))
2801+
if os.chmod in os.supports_fd:
2802+
tests.append((os.chmod, fd, 0o777))
2803+
if hasattr(os, "chown") and os.chown in os.supports_fd:
2804+
tests.append((os.chown, fd, 0, 0))
2805+
if os.listdir in os.supports_fd:
2806+
tests.append((os.listdir, fd))
2807+
if os.utime in os.supports_fd:
2808+
tests.append((os.utime, fd, (0, 0)))
2809+
if hasattr(os, "truncate") and os.truncate in os.supports_fd:
2810+
tests.append((os.truncate, fd, 0))
2811+
if hasattr(os, 'statvfs') and os.statvfs in os.supports_fd:
2812+
tests.append((os.statvfs, fd))
2813+
if hasattr(os, "setxattr"):
2814+
tests.append((os.getxattr, fd, b"user.test"))
2815+
tests.append((os.setxattr, fd, b"user.test", b"1"))
2816+
tests.append((os.removexattr, fd, b"user.test"))
2817+
tests.append((os.listxattr, fd))
2818+
if os.scandir in os.supports_fd:
2819+
tests.append((os.scandir, fd))
2820+
2821+
for func, *args in tests:
2822+
with self.subTest(func=func, args=args):
2823+
with self.assertRaises(OSError) as ctx:
2824+
func(*args)
2825+
self.assertEqual(ctx.exception.errno, errno.EBADF)
2826+
2827+
if hasattr(os, "execve") and os.execve in os.supports_fd:
2828+
# glibc fails with EINVAL, musl fails with EBADF
2829+
with self.assertRaises(OSError) as ctx:
2830+
os.execve(fd, [sys.executable, "-c", "pass"], os.environ)
2831+
self.assertIn(ctx.exception.errno, (errno.EBADF, errno.EINVAL))
2832+
2833+
if support.MS_WINDOWS:
2834+
import nt
2835+
self.assertFalse(nt._path_exists(fd))
2836+
self.assertFalse(nt._path_lexists(fd))
2837+
self.assertFalse(nt._path_isdir(fd))
2838+
self.assertFalse(nt._path_isfile(fd))
2839+
self.assertFalse(nt._path_islink(fd))
2840+
self.assertFalse(nt._path_isjunction(fd))
2841+
27912842
@unittest.skipUnless(hasattr(os, 'ftruncate'), 'test needs os.ftruncate()')
27922843
def test_ftruncate(self):
27932844
self.check(os.truncate, 0)
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
``os.listdir(-1)`` and ``os.scandir(-1)`` now fail with
2+
``OSError(errno.EBADF)`` rather than listing the current directory.
3+
``os.listxattr(-1)`` now fails with ``OSError(errno.EBADF)`` rather than
4+
listing extended attributes of the current directory. Patch by Victor
5+
Stinner.

Modules/posixmodule.c

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1638,10 +1638,10 @@ dir_fd_and_fd_invalid(const char *function_name, int dir_fd, int fd)
16381638
}
16391639

16401640
static int
1641-
fd_and_follow_symlinks_invalid(const char *function_name, int fd,
1641+
fd_and_follow_symlinks_invalid(const char *function_name, int is_fd,
16421642
int follow_symlinks)
16431643
{
1644-
if ((fd >= 0) && (!follow_symlinks)) {
1644+
if (is_fd && (!follow_symlinks)) {
16451645
PyErr_Format(PyExc_ValueError,
16461646
"%s: cannot use fd and follow_symlinks together",
16471647
function_name);
@@ -2880,12 +2880,13 @@ posix_do_stat(PyObject *module, const char *function_name, path_t *path,
28802880

28812881
if (path_and_dir_fd_invalid("stat", path, dir_fd) ||
28822882
dir_fd_and_fd_invalid("stat", dir_fd, path->fd) ||
2883-
fd_and_follow_symlinks_invalid("stat", path->fd, follow_symlinks))
2883+
fd_and_follow_symlinks_invalid("stat", path->is_fd, follow_symlinks))
28842884
return NULL;
28852885

28862886
Py_BEGIN_ALLOW_THREADS
2887-
if (path->fd != -1)
2887+
if (path->is_fd) {
28882888
result = FSTAT(path->fd, &st);
2889+
}
28892890
#ifdef MS_WINDOWS
28902891
else if (follow_symlinks)
28912892
result = win32_stat(path->wide, &st);
@@ -3647,7 +3648,7 @@ os_statx_impl(PyObject *module, path_t *path, unsigned int mask, int flags,
36473648
{
36483649
if (path_and_dir_fd_invalid("statx", path, dir_fd) ||
36493650
dir_fd_and_fd_invalid("statx", dir_fd, path->fd) ||
3650-
fd_and_follow_symlinks_invalid("statx", path->fd, follow_symlinks)) {
3651+
fd_and_follow_symlinks_invalid("statx", path->is_fd, follow_symlinks)) {
36513652
return NULL;
36523653
}
36533654

@@ -3677,7 +3678,7 @@ os_statx_impl(PyObject *module, path_t *path, unsigned int mask, int flags,
36773678

36783679
int result;
36793680
Py_BEGIN_ALLOW_THREADS
3680-
if (path->fd != -1) {
3681+
if (path->is_fd) {
36813682
result = statx(path->fd, "", flags | AT_EMPTY_PATH, mask, &v->stx);
36823683
}
36833684
else {
@@ -3934,7 +3935,7 @@ os_chdir_impl(PyObject *module, path_t *path)
39343935
result = !win32_wchdir(path->wide);
39353936
#else
39363937
#ifdef HAVE_FCHDIR
3937-
if (path->fd != -1)
3938+
if (path->is_fd)
39383939
result = fchdir(path->fd);
39393940
else
39403941
#endif
@@ -4090,7 +4091,7 @@ os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd,
40904091
#ifdef MS_WINDOWS
40914092
result = 0;
40924093
Py_BEGIN_ALLOW_THREADS
4093-
if (path->fd != -1) {
4094+
if (path->is_fd) {
40944095
result = win32_fchmod(path->fd, mode);
40954096
}
40964097
else if (follow_symlinks) {
@@ -4113,8 +4114,9 @@ os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd,
41134114
#else /* MS_WINDOWS */
41144115
Py_BEGIN_ALLOW_THREADS
41154116
#ifdef HAVE_FCHMOD
4116-
if (path->fd != -1)
4117+
if (path->is_fd) {
41174118
result = fchmod(path->fd, mode);
4119+
}
41184120
else
41194121
#endif /* HAVE_CHMOD */
41204122
#ifdef HAVE_LCHMOD
@@ -4511,7 +4513,7 @@ os_chown_impl(PyObject *module, path_t *path, uid_t uid, gid_t gid,
45114513
return NULL;
45124514
#endif
45134515
if (dir_fd_and_fd_invalid("chown", dir_fd, path->fd) ||
4514-
fd_and_follow_symlinks_invalid("chown", path->fd, follow_symlinks))
4516+
fd_and_follow_symlinks_invalid("chown", path->is_fd, follow_symlinks))
45154517
return NULL;
45164518

45174519
if (PySys_Audit("os.chown", "OIIi", path->object, uid, gid,
@@ -4521,7 +4523,7 @@ os_chown_impl(PyObject *module, path_t *path, uid_t uid, gid_t gid,
45214523

45224524
Py_BEGIN_ALLOW_THREADS
45234525
#ifdef HAVE_FCHOWN
4524-
if (path->fd != -1)
4526+
if (path->is_fd)
45254527
result = fchown(path->fd, uid, gid);
45264528
else
45274529
#endif
@@ -4999,7 +5001,7 @@ _posix_listdir(path_t *path, PyObject *list)
49995001

50005002
errno = 0;
50015003
#ifdef HAVE_FDOPENDIR
5002-
if (path->fd != -1) {
5004+
if (path->is_fd) {
50035005
if (HAVE_FDOPENDIR_RUNTIME) {
50045006
/* closedir() closes the FD, so we duplicate it */
50055007
fd = _Py_dup(path->fd);
@@ -5898,7 +5900,7 @@ _testFileExists(path_t *path, BOOL followLinks)
58985900
}
58995901

59005902
Py_BEGIN_ALLOW_THREADS
5901-
if (path->fd != -1) {
5903+
if (path->is_fd) {
59025904
HANDLE hfile = _Py_get_osfhandle_noraise(path->fd);
59035905
if (hfile != INVALID_HANDLE_VALUE) {
59045906
if (GetFileType(hfile) != FILE_TYPE_UNKNOWN || !GetLastError()) {
@@ -5924,7 +5926,7 @@ _testFileType(path_t *path, int testedType)
59245926
}
59255927

59265928
Py_BEGIN_ALLOW_THREADS
5927-
if (path->fd != -1) {
5929+
if (path->is_fd) {
59285930
HANDLE hfile = _Py_get_osfhandle_noraise(path->fd);
59295931
if (hfile != INVALID_HANDLE_VALUE) {
59305932
result = _testFileTypeByHandle(hfile, testedType, TRUE);
@@ -7141,7 +7143,7 @@ os_utime_impl(PyObject *module, path_t *path, PyObject *times, PyObject *ns,
71417143

71427144
if (path_and_dir_fd_invalid("utime", path, dir_fd) ||
71437145
dir_fd_and_fd_invalid("utime", dir_fd, path->fd) ||
7144-
fd_and_follow_symlinks_invalid("utime", path->fd, follow_symlinks))
7146+
fd_and_follow_symlinks_invalid("utime", path->is_fd, follow_symlinks))
71457147
return NULL;
71467148

71477149
#if !defined(HAVE_UTIMENSAT)
@@ -7200,7 +7202,7 @@ os_utime_impl(PyObject *module, path_t *path, PyObject *times, PyObject *ns,
72007202
#endif
72017203

72027204
#if defined(HAVE_FUTIMES) || defined(HAVE_FUTIMENS)
7203-
if (path->fd != -1)
7205+
if (path->is_fd)
72047206
result = utime_fd(&utime, path->fd);
72057207
else
72067208
#endif
@@ -7569,7 +7571,7 @@ os_execve_impl(PyObject *module, path_t *path, PyObject *argv, PyObject *env)
75697571

75707572
_Py_BEGIN_SUPPRESS_IPH
75717573
#ifdef HAVE_FEXECVE
7572-
if (path->fd > -1)
7574+
if (path->is_fd)
75737575
fexecve(path->fd, argvlist, envlist);
75747576
else
75757577
#endif
@@ -13355,7 +13357,7 @@ os_truncate_impl(PyObject *module, path_t *path, Py_off_t length)
1335513357
int fd;
1335613358
#endif
1335713359

13358-
if (path->fd != -1)
13360+
if (path->is_fd)
1335913361
return os_ftruncate_impl(module, path->fd, length);
1336013362

1336113363
if (PySys_Audit("os.truncate", "On", path->object, length) < 0) {
@@ -14052,7 +14054,7 @@ os_statvfs_impl(PyObject *module, path_t *path)
1405214054
struct statfs st;
1405314055

1405414056
Py_BEGIN_ALLOW_THREADS
14055-
if (path->fd != -1) {
14057+
if (path->is_fd) {
1405614058
result = fstatfs(path->fd, &st);
1405714059
}
1405814060
else
@@ -14070,7 +14072,7 @@ os_statvfs_impl(PyObject *module, path_t *path)
1407014072

1407114073
Py_BEGIN_ALLOW_THREADS
1407214074
#ifdef HAVE_FSTATVFS
14073-
if (path->fd != -1) {
14075+
if (path->is_fd) {
1407414076
result = fstatvfs(path->fd, &st);
1407514077
}
1407614078
else
@@ -15410,7 +15412,7 @@ os_getxattr_impl(PyObject *module, path_t *path, path_t *attribute,
1541015412
int follow_symlinks)
1541115413
/*[clinic end generated code: output=5f2f44200a43cff2 input=025789491708f7eb]*/
1541215414
{
15413-
if (fd_and_follow_symlinks_invalid("getxattr", path->fd, follow_symlinks))
15415+
if (fd_and_follow_symlinks_invalid("getxattr", path->is_fd, follow_symlinks))
1541415416
return NULL;
1541515417

1541615418
if (PySys_Audit("os.getxattr", "OO", path->object, attribute->object) < 0) {
@@ -15432,7 +15434,7 @@ os_getxattr_impl(PyObject *module, path_t *path, path_t *attribute,
1543215434
void *ptr = PyBytesWriter_GetData(writer);
1543315435

1543415436
Py_BEGIN_ALLOW_THREADS;
15435-
if (path->fd >= 0)
15437+
if (path->is_fd)
1543615438
result = fgetxattr(path->fd, attribute->narrow, ptr, buffer_size);
1543715439
else if (follow_symlinks)
1543815440
result = getxattr(path->narrow, attribute->narrow, ptr, buffer_size);
@@ -15481,7 +15483,7 @@ os_setxattr_impl(PyObject *module, path_t *path, path_t *attribute,
1548115483
{
1548215484
ssize_t result;
1548315485

15484-
if (fd_and_follow_symlinks_invalid("setxattr", path->fd, follow_symlinks))
15486+
if (fd_and_follow_symlinks_invalid("setxattr", path->is_fd, follow_symlinks))
1548515487
return NULL;
1548615488

1548715489
if (PySys_Audit("os.setxattr", "OOy#i", path->object, attribute->object,
@@ -15490,7 +15492,7 @@ os_setxattr_impl(PyObject *module, path_t *path, path_t *attribute,
1549015492
}
1549115493

1549215494
Py_BEGIN_ALLOW_THREADS;
15493-
if (path->fd > -1)
15495+
if (path->is_fd)
1549415496
result = fsetxattr(path->fd, attribute->narrow,
1549515497
value->buf, value->len, flags);
1549615498
else if (follow_symlinks)
@@ -15534,15 +15536,15 @@ os_removexattr_impl(PyObject *module, path_t *path, path_t *attribute,
1553415536
{
1553515537
ssize_t result;
1553615538

15537-
if (fd_and_follow_symlinks_invalid("removexattr", path->fd, follow_symlinks))
15539+
if (fd_and_follow_symlinks_invalid("removexattr", path->is_fd, follow_symlinks))
1553815540
return NULL;
1553915541

1554015542
if (PySys_Audit("os.removexattr", "OO", path->object, attribute->object) < 0) {
1554115543
return NULL;
1554215544
}
1554315545

1554415546
Py_BEGIN_ALLOW_THREADS;
15545-
if (path->fd > -1)
15547+
if (path->is_fd)
1554615548
result = fremovexattr(path->fd, attribute->narrow);
1554715549
else if (follow_symlinks)
1554815550
result = removexattr(path->narrow, attribute->narrow);
@@ -15584,7 +15586,7 @@ os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks)
1558415586
const char *name;
1558515587
char *buffer = NULL;
1558615588

15587-
if (fd_and_follow_symlinks_invalid("listxattr", path->fd, follow_symlinks))
15589+
if (fd_and_follow_symlinks_invalid("listxattr", path->is_fd, follow_symlinks))
1558815590
goto exit;
1558915591

1559015592
if (PySys_Audit("os.listxattr", "(O)",
@@ -15611,7 +15613,7 @@ os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks)
1561115613
}
1561215614

1561315615
Py_BEGIN_ALLOW_THREADS;
15614-
if (path->fd > -1)
15616+
if (path->is_fd)
1561515617
length = flistxattr(path->fd, buffer, buffer_size);
1561615618
else if (follow_symlinks)
1561715619
length = listxattr(name, buffer, buffer_size);
@@ -16664,7 +16666,7 @@ DirEntry_from_posix_info(PyObject *module, path_t *path, const char *name,
1666416666
entry->stat = NULL;
1666516667
entry->lstat = NULL;
1666616668

16667-
if (path->fd != -1) {
16669+
if (path->is_fd) {
1666816670
entry->dir_fd = path->fd;
1666916671
joined_path = NULL;
1667016672
}
@@ -16689,7 +16691,7 @@ DirEntry_from_posix_info(PyObject *module, path_t *path, const char *name,
1668916691
if (!entry->name)
1669016692
goto error;
1669116693

16692-
if (path->fd != -1) {
16694+
if (path->is_fd) {
1669316695
entry->path = Py_NewRef(entry->name);
1669416696
}
1669516697
else if (!entry->path)
@@ -16813,8 +16815,9 @@ ScandirIterator_closedir(ScandirIterator *iterator)
1681316815
iterator->dirp = NULL;
1681416816
Py_BEGIN_ALLOW_THREADS
1681516817
#ifdef HAVE_FDOPENDIR
16816-
if (iterator->path.fd != -1)
16818+
if (iterator->path.is_fd) {
1681716819
rewinddir(dirp);
16820+
}
1681816821
#endif
1681916822
closedir(dirp);
1682016823
Py_END_ALLOW_THREADS
@@ -17034,7 +17037,7 @@ os_scandir_impl(PyObject *module, path_t *path)
1703417037
#else /* POSIX */
1703517038
errno = 0;
1703617039
#ifdef HAVE_FDOPENDIR
17037-
if (iterator->path.fd != -1) {
17040+
if (iterator->path.is_fd) {
1703817041
if (HAVE_FDOPENDIR_RUNTIME) {
1703917042
/* closedir() closes the FD, so we duplicate it */
1704017043
fd = _Py_dup(iterator->path.fd);

0 commit comments

Comments
 (0)