From 93805484c55ac5da2304eecb0e69356f40a89552 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 1 Aug 2025 13:46:33 -0700 Subject: [PATCH] tail: fix unlikely races with >=2 --pids MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Also, fix commentary to talk about “nonexistent” rather than “dead” processes, since the code looks for the former not the latter and the difference matters for zombies. * src/tail.c (some_writers_exist): Rename from writers_are_dead, negate the sense, don’t have a special and counterintuitive case for !nbpids, remove PIDs found not to exist, and avoid some though not all unlikely races when kernels reuse PIDs. (tail_forever): Optimize via blocking I/O even if --pid was used, so long as all the writers no longer exist. (tail_forever, tail_forever_inotify): Simplify the writers_dead logic; there is no need to have a local var to track this, since we can use pids and nbpids now. (parse_options): Also free and clear pids if !forever. --- NEWS | 3 ++ doc/coreutils.texi | 4 +-- src/tail.c | 77 ++++++++++++++++++++++++---------------------- 3 files changed, 45 insertions(+), 39 deletions(-) diff --git a/NEWS b/NEWS index 0bda0eae59..aa04c18842 100644 --- a/NEWS +++ b/NEWS @@ -60,6 +60,9 @@ GNU coreutils NEWS -*- outline -*- when the input is seekable. [bug introduced in coreutils-9.6] + 'tail --pid' avoids some unlikely races if the kernel reuses PIDs. + [bug introduced in coreutils-9.5] + tty now exits with status 4 with a special diagnostic if ttyname fails even though standard input is a tty. Formerly it quietly pretended that standard input was not a tty. diff --git a/doc/coreutils.texi b/doc/coreutils.texi index c5ecf23769..34ea700855 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -3216,8 +3216,8 @@ Size multiplier suffixes are the same as with the @option{-c} option. @opindex --pid When following by name or by descriptor, you may specify the process ID, @var{pid}, of one or more (by repeating @option{--pid}) writers of the -@var{file} arguments. Then, shortly after all the identified -processes terminate, tail will also terminate. This will +@var{file} arguments. Then, @command{tail} will exit shortly +after all the identified processes no longer exist. This will work properly only if the writers and the tailing process are running on the same machine. For example, to save the output of a build in a file and to watch the file grow, if you invoke @command{make} and @command{tail} diff --git a/src/tail.c b/src/tail.c index d154330c29..2579f4e13b 100644 --- a/src/tail.c +++ b/src/tail.c @@ -313,7 +313,7 @@ With more than one FILE, precede each with a header giving the file name.\n\ DEFAULT_MAX_N_UNCHANGED_STATS_BETWEEN_OPENS ); fputs (_("\ - --pid=PID with -f, terminate after process ID, PID dies;\n\ + --pid=PID with -f, exit after PID no longer exists;\n\ can be repeated to watch multiple processes\n\ -q, --quiet, --silent never output headers giving file names\n\ --retry keep trying to open a file if it is inaccessible\n\ @@ -1122,23 +1122,26 @@ any_live_files (const struct File_spec *f, int n_files) return false; } -/* Determine whether all watched writers are dead. - Returns true only if all processes' states can be determined, - and all processes no longer exist. */ +/* Return true if any writers exist, even as zombies. */ static bool -writers_are_dead (void) +some_writers_exist (void) { - if (!nbpids) - return false; - - for (int i = 0; i < nbpids; i++) + /* To avoid some (though not all) races if the kernel reuses PIDs, + check all PIDs instead of returning merely because one PID exists. */ + for (idx_t i = 0; i < nbpids; ) { - if (! (kill (pids[i], 0) < 0 && errno != EPERM)) - return false; + if (kill (pids[i], 0) < 0 && errno == ESRCH) + { + nbpids--; + memmove (&pids[i], &pids[i + 1], (nbpids - i) * sizeof pids[i]); + } + else + i++; } - return true; + + return 0 < nbpids; } /* Tail N_FILES files forever, or until killed. @@ -1151,14 +1154,14 @@ writers_are_dead (void) static void tail_forever (struct File_spec *f, int n_files, double sleep_interval) { - /* Use blocking I/O as an optimization, when it's easy. */ - bool blocking = (!nbpids && follow_mode == Follow_descriptor - && n_files == 1 && 0 <= f[0].fd && ! S_ISREG (f[0].mode)); - bool writers_dead = false; int last = n_files - 1; while (true) { + /* Use blocking I/O as an optimization, when it's easy. */ + bool blocking = (!nbpids && follow_mode == Follow_descriptor + && n_files == 1 && 0 <= f[0].fd && !S_ISREG (f[0].mode)); + bool any_input = false; for (int i = 0; i < n_files; i++) @@ -1296,19 +1299,21 @@ tail_forever (struct File_spec *f, int n_files, double sleep_interval) check_output_alive (); - /* If nothing was read, sleep and/or check for dead writers. */ if (!any_input) { - if (writers_dead) - break; - - /* Once the writer is dead, read the files once more to - avoid a race condition. */ - writers_dead = writers_are_dead (); + if (pids) + { + /* Exit loop if it was already known that no writers exist. + Otherwise, to avoid a race, loop one more time + without sleeping if no writers exist now. */ + if (!nbpids) + break; + if (!some_writers_exist ()) + continue; + } - if (!writers_dead && xnanosleep (sleep_interval)) + if (xnanosleep (sleep_interval)) error (EXIT_FAILURE, errno, _("cannot read realtime clock")); - } } } @@ -1462,7 +1467,6 @@ tail_forever_inotify (int wd, struct File_spec *f, int n_files, bool tailed_but_unwatchable = false; bool found_unwatchable_dir = false; bool no_inotify_resources = false; - bool writers_dead = false; struct File_spec *prev_fspec; idx_t evlen = 0; char *evbuf; @@ -1627,14 +1631,12 @@ tail_forever_inotify (int wd, struct File_spec *f, int n_files, /* How many ms to wait for changes. -1 means wait forever. */ int delay = -1; - if (nbpids) + if (pids) { - if (writers_dead) + if (!nbpids) exit (EXIT_SUCCESS); - writers_dead = writers_are_dead (); - - if (writers_dead || sleep_interval <= 0) + if (!some_writers_exist () || sleep_interval <= 0) delay = 0; else if (sleep_interval < INT_MAX / 1000 - 1) { @@ -2270,14 +2272,15 @@ parse_options (int argc, char **argv, error (0, 0, _("warning: --retry only effective for the initial open")); } - if (nbpids && !forever) - error (0, 0, - _("warning: PID ignored; --pid=PID is useful only when following")); - else if (nbpids && kill (pids[0], 0) < 0 && errno == ENOSYS) + if (pids && (!forever || (kill (pids[0], 0) < 0 && errno == ENOSYS))) { - error (0, 0, _("warning: --pid=PID is not supported on this system")); - nbpids = 0; + error (0, 0, + _(forever + ? "warning: --pid=PID is not supported on this system" + : ("warning: PID ignored;" + " --pid=PID is useful only when following"))); free (pids); + pids = nullptr; } } -- 2.47.2