diff --git a/src/comm/comm_server.cpp b/src/comm/comm_server.cpp index c8dbe1c8..b47d4432 100644 --- a/src/comm/comm_server.cpp +++ b/src/comm/comm_server.cpp @@ -381,7 +381,7 @@ namespace comm ss << "/proc/" << uc.pid << "/environ"; std::string fn = ss.str(); - const int envfd = open(fn.c_str(), O_RDONLY); + const int envfd = open(fn.c_str(), O_RDONLY | O_CLOEXEC); if (!envfd) { LOG_ERR << errno << ": Could not open environ block for process on other end of unix domain socket PID=" << uc.pid; diff --git a/src/hpfs/hpfs.cpp b/src/hpfs/hpfs.cpp index cb72a803..14a8a7d8 100644 --- a/src/hpfs/hpfs.cpp +++ b/src/hpfs/hpfs.cpp @@ -184,7 +184,7 @@ namespace hpfs int get_hash(h32 &hash, const std::string_view mount_dir, const std::string_view vpath) { const std::string path = std::string(mount_dir).append(vpath).append("::hpfs.hmap.hash"); - const int fd = open(path.c_str(), O_RDONLY); + const int fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); if (fd == -1 && errno == ENOENT) { LOG_DBG << "Cannot get hash. vpath not found. " << vpath; @@ -213,7 +213,7 @@ namespace hpfs int get_file_block_hashes(std::vector &hashes, const std::string_view mount_dir, const std::string_view vpath) { const std::string path = std::string(mount_dir).append(vpath).append("::hpfs.hmap.children"); - const int fd = open(path.c_str(), O_RDONLY); + const int fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); if (fd == -1 && errno == ENOENT) { LOG_DBG << "Cannot get file block hashes. vpath not found. " << vpath; @@ -253,7 +253,7 @@ namespace hpfs int get_dir_children_hashes(std::vector &hash_nodes, const std::string_view mount_dir, const std::string_view dir_vpath) { const std::string path = std::string(mount_dir).append(dir_vpath).append("::hpfs.hmap.children"); - const int fd = open(path.c_str(), O_RDONLY); + const int fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); if (fd == -1 && errno == ENOENT) { LOG_DBG << "Cannot get dir children hashes. Dir vpath not found. " << dir_vpath; diff --git a/src/sc.cpp b/src/sc.cpp index 0e909694..afa4cc71 100644 --- a/src/sc.cpp +++ b/src/sc.cpp @@ -514,7 +514,6 @@ namespace sc /** * Common function to create a pair of pipes (Hp->SC, SC->HP). * @param fds Vector to populate fd list. - * @param inputbuffer Buffer to write into the HP write fd. * @param create_inpipe Whether to create the input pipe from HP to SC. */ int create_iopipes(std::vector &fds, const bool create_inpipe) @@ -643,23 +642,31 @@ namespace sc /** * Common function for closing unused fds based on which process this gets called from. + * This also marks active fds with O_CLOEXEC for close-on-exec behaviour. * @param is_hp Specify 'true' when calling from HP process. 'false' from SC process. * @param fds Vector of fds to close. */ void close_unused_vectorfds(const bool is_hp, std::vector &fds) { - const int fdtypes_to_close[2] = { - is_hp ? FDTYPE::SCREAD : FDTYPE::HPREAD, - is_hp ? FDTYPE::SCWRITE : FDTYPE::HPWRITE, - }; - - for (const int fdtype : fdtypes_to_close) + for (int fd_type = 0; fd_type <= 3; fd_type++) { - const int fd = fds[fdtype]; + const int fd = fds[fd_type]; if (fd != -1) { - close(fd); - fds[fdtype] = -1; + if ((is_hp && (fd_type == FDTYPE::SCREAD || fd_type == FDTYPE::SCWRITE)) || + (!is_hp && (fd_type == FDTYPE::HPREAD || fd_type == FDTYPE::HPWRITE))) + { + close(fd); + fds[fd_type] = -1; + } + else if (is_hp && (fd_type == FDTYPE::HPREAD || fd_type == FDTYPE::HPWRITE)) + { + // The fd must be kept open in HP process. But we must + // mark it to close on exec in a potential forked process. + int flags = fcntl(fd, F_GETFD, NULL); + flags |= FD_CLOEXEC; + fcntl(fd, F_SETFD, flags); + } } } } diff --git a/src/state/state_serve.cpp b/src/state/state_serve.cpp index 54d4b38f..27a5cdb8 100644 --- a/src/state/state_serve.cpp +++ b/src/state/state_serve.cpp @@ -219,7 +219,7 @@ namespace state_serve struct stat st; const std::string file_path = std::string(mount_dir).append(vpath); const off_t block_offset = block_id * state_common::BLOCK_SIZE; - const int fd = open(file_path.c_str(), O_RDONLY); + const int fd = open(file_path.c_str(), O_RDONLY | O_CLOEXEC); if (fd == -1) { LOG_ERR << errno << ": Open failed. " << file_path; diff --git a/src/state/state_sync.cpp b/src/state/state_sync.cpp index 13633398..6c778c1c 100644 --- a/src/state/state_sync.cpp +++ b/src/state/state_sync.cpp @@ -49,10 +49,10 @@ namespace state_sync } /** - * Sets a new target state for the syncing process. - * @param target_state The target state which we should sync towards. - * @param completion_callback The callback function to call upon state sync completion. - */ + * Sets a new target state for the syncing process. + * @param target_state The target state which we should sync towards. + * @param completion_callback The callback function to call upon state sync completion. + */ void set_target(const hpfs::h32 target_state, void (*const completion_callback)(const hpfs::h32)) { std::scoped_lock lock(ctx.target_state_update_lock); @@ -67,8 +67,8 @@ namespace state_sync } /** - * Runs the state sync worker loop. - */ + * Runs the state sync worker loop. + */ void state_syncer_loop() { util::mask_signal(); @@ -238,8 +238,8 @@ namespace state_sync } /** - * Indicates whether to break out of state request processing loop. - */ + * Indicates whether to break out of state request processing loop. + */ bool should_stop_request_loop(const hpfs::h32 current_target) { if (ctx.is_shutting_down) @@ -251,12 +251,12 @@ namespace state_sync } /** - * Sends a state request to a random peer. - * @param path Requested file or dir path. - * @param is_file Whether the requested path if a file or dir. - * @param block_id The requested block id. Only relevant if requesting a file block. Otherwise -1. - * @param expected_hash The expected hash of the requested data. The peer will ignore the request if their hash is different. - */ + * Sends a state request to a random peer. + * @param path Requested file or dir path. + * @param is_file Whether the requested path if a file or dir. + * @param block_id The requested block id. Only relevant if requesting a file block. Otherwise -1. + * @param expected_hash The expected hash of the requested data. The peer will ignore the request if their hash is different. + */ void request_state_from_peer(const std::string &path, const bool is_file, const int32_t block_id, const hpfs::h32 expected_hash) { p2p::state_request sr; @@ -271,8 +271,8 @@ namespace state_sync } /** - * Submits a pending state request to the peer. - */ + * Submits a pending state request to the peer. + */ void submit_request(const backlog_item &request) { LOG_DBG << "State sync: Submitting request. type:" << request.type @@ -288,8 +288,8 @@ namespace state_sync } /** - * Process dir children response. - */ + * Process dir children response. + */ int handle_fs_entry_response(std::string_view parent_vpath, const msg::fbuf::p2pmsg::Fs_Entry_Response *fs_entry_resp) { // Get the parent path of the fs entries we have received. @@ -364,8 +364,8 @@ namespace state_sync } /** - * Process file block hash map response. - */ + * Process file block hash map response. + */ int handle_file_hashmap_response(std::string_view file_vpath, const msg::fbuf::p2pmsg::File_HashMap_Response *file_resp) { // Get the file path of the block hashes we have received. @@ -403,8 +403,8 @@ namespace state_sync } /** - * Process file block response. - */ + * Process file block response. + */ int handle_file_block_response(std::string_view file_vpath, const msg::fbuf::p2pmsg::Block_Response *block_msg) { // Get the file path of the block data we have received. @@ -416,7 +416,7 @@ namespace state_sync << ") of " << file_vpath; std::string file_physical_path = std::string(ctx.hpfs_mount_dir).append(file_vpath); - const int fd = open(file_physical_path.c_str(), O_WRONLY | O_CREAT, FILE_PERMS); + const int fd = open(file_physical_path.c_str(), O_WRONLY | O_CREAT | O_CLOEXEC, FILE_PERMS); if (fd == -1) { LOG_ERR << errno << " Open failed " << file_physical_path;