Skip to content

Commit

Permalink
Fix race in libzfs_run_process_impl
Browse files Browse the repository at this point in the history
When replacing a disk, a child process is forked to run a script called
zfs_prepare_disk (which can be useful for disk firmware update or health
check). The parent than calls waitpid and checks the child error/status
code.

However, the _reap_children thread (created from zed_exec_process to
manage zedlets) also waits for all children with the same PGID and can
stole the signal, causing the replace operation to be aborted.

As waitpid returns -1, the parent incorrectly assume that the child
process had an error or was killed. This, in turn, leaves the newly
added disk in REMOVED or UNAVAIL status rather than completing the
replace process.

This patch changes the PGID of the child process execuing the
prepare script, shielding it from the _reap_children thread.

Signed-off-by: Gionatan Danti <g.danti@assyoma.it>
  • Loading branch information
shodanshok committed Dec 2, 2024
1 parent 38c0324 commit 72cea01
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/libzfs/libzfs_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,7 @@ libzfs_run_process_impl(const char *path, char *argv[], char *env[], int flags,
pid = fork();
if (pid == 0) {
/* Child process */
setpgid (0, 0);
devnull_fd = open("/dev/null", O_WRONLY | O_CLOEXEC);

if (devnull_fd < 0)
Expand Down Expand Up @@ -967,7 +968,7 @@ libzfs_run_process_impl(const char *path, char *argv[], char *env[], int flags,
while ((error = waitpid(pid, &status, 0)) == -1 &&
errno == EINTR)
;
if (error < 0 || !WIFEXITED(status))
if (!WIFEXITED(status))
return (-1);

if (lines != NULL) {
Expand Down

0 comments on commit 72cea01

Please sign in to comment.