From c014c23662f77498cd69b2c5e99ec9bdba9c9a3a Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 17 Aug 2021 19:52:15 -0500 Subject: [PATCH] Fix undefined behavior in closing a moved pipe `pipe_next_read` is moved in the body of the loop, and not re-initialized the last go around. However, we call `pipe_next_read.close()` after the loop, which is undefined behavior (as it's been moved). Best case scenario, the compiler passed the address of our copy of the struct to `exec_process_in_job` and beyond, it went out of scope there, the value of `fd` was set to closed (minus one), and we explicitly call `.close()` again, in which case it does nothing. Worst case scenario, the compiler re-uses the storage for the now-moved struct for something else and our call to `.close()` ends up closing some other value of `fd` (valid or invalid) and things break. Aside from the fact that we obviously don't need to close it since it's not assigned for the last process in the job, it's a RAII object so we don't have to worry about manually closing it in the first place. --- src/exec.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/exec.cpp b/src/exec.cpp index 119553f03..aba8d6378 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1093,7 +1093,6 @@ bool exec_job(parser_t &parser, const shared_ptr &j, const io_chain_t &bl } procs_launched += 1; } - pipe_next_read.close(); // If our pipeline was aborted before any process was successfully launched, then there is // nothing to reap, and we can perform an early return.