When FTS_TIGHT_CYCLE_CHECK is enabled, fts_build() leaves directory and then re-enters it again with updated stat information, which leads to double hash removal of the same directory in a case the second call to fstat() fails as then the directory is not reentered, but the caller of fts_build(), the function fts_add(), stil removes it upon fts_build() return. This commonly happens on /proc filesystem where permission to stat process files and directories depends on that process euid and ptraceability configuration.
Rework the code to not remove the old entry until the new entry is successfully inserted to the hash table, as a similar problem could arise also when the updated entry insertion fails (on a cycle or memory allocation failure). --- ChangeLog | 18 +++++++++++ lib/fts.c | 33 ++++++++++++++----- modules/fts-tests | 5 +-- tests/test-fts-2.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 10 deletions(-) create mode 100644 tests/test-fts-2.c diff --git a/ChangeLog b/ChangeLog index 6084329d2e..5fe8813190 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,21 @@ +2025-08-15 Petr Malat <[email protected]> + + fts: Avoid crash if fstat() fails + + When FTS_TIGHT_CYCLE_CHECK is enabled, fts_build() leaves directory and + then re-enters it again with updated stat information, which leads to + double hash removal of the same directory in a case the second call to + fstat() fails as then the directory is not reentered, but the caller + of fts_build(), the function fts_add(), stil removes it upon fts_build() + return. This commonly happens on /proc filesystem where permission to + stat process files and directories depends on that process euid and + ptraceability configuration. + + Rework the code to not remove the old entry until the new entry is + successfully inserted to the hash table, as a similar problem could + arise also when the updated entry insertion fails (on a cycle or memory + allocation failure). + 2025-08-11 Paul Eggert <[email protected]> manywarnings: update C warnings for GCC 15.2 diff --git a/lib/fts.c b/lib/fts.c index b611e997d4..cef5e38812 100644 --- a/lib/fts.c +++ b/lib/fts.c @@ -1326,9 +1326,8 @@ fts_build (register FTS *sp, int type) benefit/suffer from this feature for now. */ || ISSET (FTS_TIGHT_CYCLE_CHECK)) { - if (!stat_optimization) - LEAVE_DIR (sp, cur, "4"); - if (fstat (dir_fd, cur->fts_statp) != 0) + struct stat statbuf; + if (fstat (dir_fd, &statbuf) != 0) { int fstat_errno = errno; closedir_and_clear (cur->fts_dirp); @@ -1342,13 +1341,31 @@ fts_build (register FTS *sp, int type) } if (stat_optimization) cur->fts_info = FTS_D; - else if (! enter_dir (sp, cur)) + else if (statbuf.st_ino != cur->fts_statp->st_ino || + statbuf.st_dev != cur->fts_statp->st_dev) { - int err = errno; - closedir_and_clear (cur->fts_dirp); - __set_errno (err); - return NULL; + struct stat stattmp; + bool enter_ok; + + stattmp = *cur->fts_statp; + *cur->fts_statp = statbuf; + enter_ok = enter_dir (sp, cur); + *cur->fts_statp = stattmp; + + /* leave_dir must not be called until the updated directory + is successfully entered, because caller of this function + calls leave_dir as well. */ + if (enter_ok) + LEAVE_DIR (sp, cur, "4"); + else + { + int err = errno; + closedir_and_clear (cur->fts_dirp); + __set_errno (err); + return NULL; + } } + *cur->fts_statp = statbuf; } } diff --git a/modules/fts-tests b/modules/fts-tests index 5feb3c3c6e..02e91aeb95 100644 --- a/modules/fts-tests +++ b/modules/fts-tests @@ -3,6 +3,7 @@ longrunning-test Files: tests/test-fts.c +tests/test-fts-2.c Depends-on: c99 @@ -13,6 +14,6 @@ unlinkat configure.ac: Makefile.am: -TESTS += test-fts -check_PROGRAMS += test-fts +TESTS += test-fts test-fts-2 +check_PROGRAMS += test-fts test-fts-2 test_fts_LDADD = $(LDADD) @LIBINTL@ diff --git a/tests/test-fts-2.c b/tests/test-fts-2.c new file mode 100644 index 0000000000..915354997e --- /dev/null +++ b/tests/test-fts-2.c @@ -0,0 +1,81 @@ +/* Test the fts_read() properly handles fstat() failure. + + Simulate fstat() failure by providing custom fstat() implementation. + The test is enabled only on glibc, as I'm not sure if fstat is a weak + symbol on all supported platforms or even if week symbols exist on + them. + + Note that fts_read() calls fstatat() prior the call to fstat(), which + is expected to pass to simulate the scenario which crashed find while + searching /proc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <https://www.gnu.org/licenses/>. */ + +#include <config.h> +#include <features.h> +#include <stdio.h> + +#if defined(__GLIBC__) && __GLIBC__ >= 2 + +#include <fts_.h> + +#include <string.h> +#include <dlfcn.h> +#include <errno.h> + +#include "macros.h" + +#define TEST_PATH "/tmp" + +static unsigned fstat_counter; + +int +fstat (int fd, struct stat *statbuf) +{ + fstat_counter++; + return -ENOENT; +} + +int +main (void) +{ + char *fl[2] = {TEST_PATH, NULL}; + FTSENT *e; + FTS *p; + + // Use the same flags as find, where the race has been discovered + p = fts_open (fl, FTS_NOSTAT | FTS_TIGHT_CYCLE_CHECK | FTS_CWDFD | FTS_VERBATIM | FTS_LOGICAL, NULL); + ASSERT (p); + + while ((e = fts_read (p)) != NULL) + { + // The directory shouldn't be entered + ASSERT (strcmp (e->fts_name, basename(TEST_PATH)) == 0); + } + + ASSERT (fts_close (p) == 0); + ASSERT (fstat_counter == 1); + return test_exit_status; +} + +#else + +int +main (void) +{ + fprintf (stderr, "Skipping test: unsupported system\n"); + return 77; +} + +#endif -- 2.39.5
