Skip to content

gh-81441: shutil.rmtree() FileNotFoundError race condition #14064

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

Merged
merged 21 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
03d4a2d
bpo-37260: ignore missing files and directories while enumerating
websurfer5 Jun 13, 2019
2fd3869
Merge branch 'master' of github.com:python/cpython into fix-issue-37260
websurfer5 Jun 13, 2019
e7b5cc5
bpo-37260: use os.path to build paths in tests; abort shutil.rmtree()
websurfer5 Jun 14, 2019
1d960dc
Merge branch 'fix-issue-37260' of github.com:websurfer5/cpython into …
websurfer5 Jun 14, 2019
b56f2a4
Merge branch 'master' of github.com:python/cpython into fix-issue-37260
websurfer5 Jun 14, 2019
39d04de
bpo-37260: fix whitespace issues
websurfer5 Jun 14, 2019
8268448
bpo-37260: fix whitespace issues
websurfer5 Jun 14, 2019
5a9da9c
📜🤖 Added by blurb_it.
blurb-it[bot] Jun 14, 2019
e1e386b
Merge branch 'fix-issue-37260' of github.com:websurfer5/cpython into …
websurfer5 Jun 14, 2019
13b31e1
Merge branch 'master' of github.com:python/cpython into fix-issue-37260
websurfer5 Jun 14, 2019
905f649
bpo-37260: continue, not pass, within loops
websurfer5 Jun 15, 2019
d6789d7
Update Lib/shutil.py
websurfer5 Jun 25, 2019
561047c
bpo-37260: return immediately if path is missing instead of falling
websurfer5 Jun 25, 2019
bb6d000
Merge branch 'main' into fix-issue-37260
serhiy-storchaka Dec 1, 2023
a9273a8
Fix tests.
serhiy-storchaka Dec 1, 2023
d3fbad7
Update the NEWS entry.
serhiy-storchaka Dec 2, 2023
9b0eb77
Use onexc instead of onerror in tests.
serhiy-storchaka Dec 2, 2023
05c626f
Merge exists() and islink() checks.
serhiy-storchaka Dec 3, 2023
dac6e39
Rewrite tests.
serhiy-storchaka Dec 4, 2023
45c73dc
Merge branch 'main' into fix-issue-37260
serhiy-storchaka Dec 4, 2023
dd731da
Documentation.
serhiy-storchaka Dec 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Doc/library/shutil.rst
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,10 @@ Directory and files operations
.. versionchanged:: 3.12
Added the *onexc* parameter, deprecated *onerror*.

.. versionchanged:: 3.13
:func:`!rmtree` now ignores :exc:`FileNotFoundError` exceptions for all
but the top-level path.

.. attribute:: rmtree.avoids_symlink_attacks

Indicates whether the current platform and implementation provides a
Expand Down
45 changes: 34 additions & 11 deletions Lib/shutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,30 +590,30 @@ def copytree(src, dst, symlinks=False, ignore=None, copy_function=copy2,
dirs_exist_ok=dirs_exist_ok)

if hasattr(os.stat_result, 'st_file_attributes'):
def _rmtree_islink(path):
try:
st = os.lstat(path)
return (stat.S_ISLNK(st.st_mode) or
(st.st_file_attributes & stat.FILE_ATTRIBUTE_REPARSE_POINT
and st.st_reparse_tag == stat.IO_REPARSE_TAG_MOUNT_POINT))
except OSError:
return False
def _rmtree_islink(st):
return (stat.S_ISLNK(st.st_mode) or
(st.st_file_attributes & stat.FILE_ATTRIBUTE_REPARSE_POINT
and st.st_reparse_tag == stat.IO_REPARSE_TAG_MOUNT_POINT))
else:
def _rmtree_islink(path):
return os.path.islink(path)
def _rmtree_islink(st):
return stat.S_ISLNK(st.st_mode)

# version vulnerable to race conditions
def _rmtree_unsafe(path, onexc):
try:
with os.scandir(path) as scandir_it:
entries = list(scandir_it)
except FileNotFoundError:
return
except OSError as err:
onexc(os.scandir, path, err)
entries = []
for entry in entries:
fullname = entry.path
try:
is_dir = entry.is_dir(follow_symlinks=False)
except FileNotFoundError:
continue
except OSError:
is_dir = False

Expand All @@ -624,17 +624,23 @@ def _rmtree_unsafe(path, onexc):
# a directory with a symlink after the call to
# os.scandir or entry.is_dir above.
raise OSError("Cannot call rmtree on a symbolic link")
except FileNotFoundError:
continue
except OSError as err:
onexc(os.path.islink, fullname, err)
continue
_rmtree_unsafe(fullname, onexc)
else:
try:
os.unlink(fullname)
except FileNotFoundError:
continue
except OSError as err:
onexc(os.unlink, fullname, err)
try:
os.rmdir(path)
except FileNotFoundError:
pass
except OSError as err:
onexc(os.rmdir, path, err)

Expand All @@ -643,6 +649,8 @@ def _rmtree_safe_fd(topfd, path, onexc):
try:
with os.scandir(topfd) as scandir_it:
entries = list(scandir_it)
except FileNotFoundError:
return
except OSError as err:
err.filename = path
onexc(os.scandir, path, err)
Expand All @@ -651,20 +659,26 @@ def _rmtree_safe_fd(topfd, path, onexc):
fullname = os.path.join(path, entry.name)
try:
is_dir = entry.is_dir(follow_symlinks=False)
except FileNotFoundError:
continue
except OSError:
is_dir = False
else:
if is_dir:
try:
orig_st = entry.stat(follow_symlinks=False)
is_dir = stat.S_ISDIR(orig_st.st_mode)
except FileNotFoundError:
continue
except OSError as err:
onexc(os.lstat, fullname, err)
continue
if is_dir:
try:
dirfd = os.open(entry.name, os.O_RDONLY, dir_fd=topfd)
dirfd_closed = False
except FileNotFoundError:
continue
except OSError as err:
onexc(os.open, fullname, err)
else:
Expand All @@ -675,6 +689,8 @@ def _rmtree_safe_fd(topfd, path, onexc):
os.close(dirfd)
dirfd_closed = True
os.rmdir(entry.name, dir_fd=topfd)
except FileNotFoundError:
continue
except OSError as err:
onexc(os.rmdir, fullname, err)
else:
Expand All @@ -692,6 +708,8 @@ def _rmtree_safe_fd(topfd, path, onexc):
else:
try:
os.unlink(entry.name, dir_fd=topfd)
except FileNotFoundError:
continue
except OSError as err:
onexc(os.unlink, fullname, err)

Expand Down Expand Up @@ -781,7 +799,12 @@ def onexc(*args):
if dir_fd is not None:
raise NotImplementedError("dir_fd unavailable on this platform")
try:
if _rmtree_islink(path):
st = os.lstat(path)
except OSError as err:
onexc(os.lstat, path, err)
return
try:
if _rmtree_islink(st):
# symlinks to directories are forbidden, see bug #1669
raise OSError("Cannot call rmtree on a symbolic link")
except OSError as err:
Expand Down
57 changes: 57 additions & 0 deletions Lib/test/test_shutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,63 @@ def test_rmtree_on_junction(self):
finally:
shutil.rmtree(TESTFN, ignore_errors=True)

@unittest.skipIf(sys.platform[:6] == 'cygwin',
"This test can't be run on Cygwin (issue #1071513).")
@os_helper.skip_if_dac_override
@os_helper.skip_unless_working_chmod
def test_rmtree_deleted_race_condition(self):
# bpo-37260
#
# Test that a file or a directory deleted after it is enumerated
# by scandir() but before unlink() or rmdr() is called doesn't
# generate any errors.
def _onexc(fn, path, exc):
assert fn in (os.rmdir, os.unlink)
if not isinstance(exc, PermissionError):
raise
# Make the parent and the children writeable.
for p, mode in zip(paths, old_modes):
os.chmod(p, mode)
# Remove other dirs except one.
keep = next(p for p in dirs if p != path)
for p in dirs:
if p != keep:
os.rmdir(p)
# Remove other files except one.
keep = next(p for p in files if p != path)
for p in files:
if p != keep:
os.unlink(p)

os.mkdir(TESTFN)
paths = [TESTFN] + [os.path.join(TESTFN, f'child{i}')
for i in range(6)]
dirs = paths[1::2]
files = paths[2::2]
for path in dirs:
os.mkdir(path)
for path in files:
write_file(path, '')

old_modes = [os.stat(path).st_mode for path in paths]

# Make the parent and the children non-writeable.
new_mode = stat.S_IREAD|stat.S_IEXEC
for path in reversed(paths):
os.chmod(path, new_mode)

try:
shutil.rmtree(TESTFN, onexc=_onexc)
except:
# Test failed, so cleanup artifacts.
for path, mode in zip(paths, old_modes):
try:
os.chmod(path, mode)
except OSError:
pass
shutil.rmtree(TESTFN)
raise


class TestCopyTree(BaseTest, unittest.TestCase):

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed a race condition in :func:`shutil.rmtree` in which directory entries removed by another process or thread while ``shutil.rmtree()`` is running can cause it to raise FileNotFoundError. Patch by Jeffrey Kintscher.