From 8316ff96ce7f607bd74891088ee87409467dc5fa Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Sun, 31 Dec 2017 13:19:52 +0000 Subject: [PATCH] Close unused ends of BFD pipe, and streamline pipe opening code Signed-off-by: Quentin Armitage --- keepalived/bfd/bfd_daemon.c | 9 +++++---- keepalived/core/main.c | 2 +- keepalived/vrrp/vrrp_daemon.c | 8 ++++++++ keepalived/vrrp/vrrp_dbus.c | 29 +++++++---------------------- lib/signals.c | 19 +------------------ lib/utils.c | 24 ++++++++++++++++++++++++ lib/utils.h | 3 +++ 7 files changed, 49 insertions(+), 45 deletions(-) diff --git a/keepalived/bfd/bfd_daemon.c b/keepalived/bfd/bfd_daemon.c index cb5acdd24..a0cf976a9 100644 --- a/keepalived/bfd/bfd_daemon.c +++ b/keepalived/bfd/bfd_daemon.c @@ -46,6 +46,7 @@ #include "utils.h" #include "scheduler.h" #include "process.h" +#include "utils.h" /* Global variables */ int bfd_event_pipe[2] = { -1, -1}; @@ -101,13 +102,10 @@ void open_bfd_pipe(void) { /* Open BFD control pipe */ - if (pipe(bfd_event_pipe) == -1) { + if (open_pipe(bfd_event_pipe) == -1) { log_message(LOG_ERR, "Unable to create BFD event pipe: %m"); stop_keepalived(); - return; } - fcntl(bfd_event_pipe[0], F_SETFL, O_NONBLOCK | fcntl(bfd_event_pipe[0], F_GETFL)); - fcntl(bfd_event_pipe[1], F_SETFL, O_NONBLOCK | fcntl(bfd_event_pipe[1], F_GETFL)); } /* Daemon init sequence */ @@ -266,6 +264,9 @@ start_bfd_child(void) prog_type = PROG_TYPE_BFD; + /* Close the read end of the event notification pipe */ + close(bfd_event_pipe[0]); + if ((instance_name #if HAVE_DECL_CLONE_NEWNET || network_namespace diff --git a/keepalived/core/main.c b/keepalived/core/main.c index 7811df888..eab506e92 100644 --- a/keepalived/core/main.c +++ b/keepalived/core/main.c @@ -385,7 +385,7 @@ static void start_keepalived(void) { #ifdef _WITH_BFD_ - /* must be opened before vrrp and bfd */ + /* must be opened before vrrp and bfd start */ open_bfd_pipe(); #endif diff --git a/keepalived/vrrp/vrrp_daemon.c b/keepalived/vrrp/vrrp_daemon.c index 8ac942569..48ea85f59 100644 --- a/keepalived/vrrp/vrrp_daemon.c +++ b/keepalived/vrrp/vrrp_daemon.c @@ -68,6 +68,9 @@ #ifdef _WITH_JSON_ #include "vrrp_json.h" #endif +#ifdef _WITH_BFD_ +#include "bfd_daemon.h" +#endif /* Global variables */ bool non_existent_interface_specified; @@ -564,6 +567,11 @@ start_vrrp_child(void) prog_type = PROG_TYPE_VRRP; +#ifdef _WITH_BFD_ + /* Close the write end of the BFD event notification pipe */ + close(bfd_event_pipe[1]); +#endif + /* Opening local VRRP syslog channel */ if ((instance_name #if HAVE_DECL_CLONE_NEWNET diff --git a/keepalived/vrrp/vrrp_dbus.c b/keepalived/vrrp/vrrp_dbus.c index 49d4e2f12..8c4cc4a5d 100644 --- a/keepalived/vrrp/vrrp_dbus.c +++ b/keepalived/vrrp/vrrp_dbus.c @@ -54,6 +54,7 @@ #include "global_data.h" #include "main.h" #include "logger.h" +#include "utils.h" typedef enum dbus_action { DBUS_ACTION_NONE, @@ -859,37 +860,21 @@ dbus_start(void) pthread_t dbus_thread; sigset_t sigset, cursigset; -#ifdef HAVE_PIPE2 - if (pipe2(dbus_in_pipe, O_CLOEXEC)) { + if (open_pipe(dbus_in_pipe)) { log_message(LOG_INFO, "Unable to create inbound dbus pipe - disabling DBus"); return false; } - if (pipe2(dbus_out_pipe, O_CLOEXEC)) { + if (open_pipe(dbus_out_pipe)) { log_message(LOG_INFO, "Unable to create outbound dbus pipe - disabling DBus"); close(dbus_in_pipe[0]); close(dbus_in_pipe[1]); return false; } -#else - if (pipe(dbus_in_pipe)) { - log_message(LOG_INFO, "Unable to create inbound dbus pipe - disabling DBus"); - return false; - } - if (pipe(dbus_out_pipe)) { - log_message(LOG_INFO, "Unable to create outbound dbus pipe - disabling DBus"); - close(dbus_in_pipe[0]); - close(dbus_in_pipe[1]); - return false; - } - fcntl (dbus_in_pipe[0], F_SETFD, FD_CLOEXEC | fcntl(dbus_in_pipe[0], F_GETFD)); - fcntl (dbus_in_pipe[1], F_SETFD, FD_CLOEXEC | fcntl(dbus_in_pipe[1], F_GETFD)); - fcntl (dbus_out_pipe[0], F_SETFD, FD_CLOEXEC | fcntl(dbus_out_pipe[0], F_GETFD)); - fcntl (dbus_out_pipe[1], F_SETFD, FD_CLOEXEC | fcntl(dbus_out_pipe[1], F_GETFD)); -#endif - /* We don't want the main thread to block when using the pipes */ - fcntl(dbus_in_pipe[0], F_SETFL, O_NONBLOCK | fcntl(dbus_in_pipe[0], F_GETFL)); - fcntl(dbus_out_pipe[1], F_SETFL, O_NONBLOCK | fcntl(dbus_out_pipe[1], F_GETFL)); + /* We don't want the main thread to block when using the pipes, + * but the other side of the pipes should block. */ + fcntl(dbus_in_pipe[1], F_SETFL, fcntl(dbus_in_pipe[1], F_GETFL) & ~O_NONBLOCK); + fcntl(dbus_out_pipe[0], F_SETFL, fcntl(dbus_out_pipe[0], F_GETFL) & ~O_NONBLOCK); thread_add_read(master, handle_dbus_msg, NULL, dbus_in_pipe[0], TIMER_NEVER); diff --git a/lib/signals.c b/lib/signals.c index fcb9b73a6..d89bd64e1 100644 --- a/lib/signals.c +++ b/lib/signals.c @@ -343,25 +343,8 @@ open_signal_fd(void) if (signal_fd == -1) log_message(LOG_INFO, "BUG - signal_fd init failed - %d (%s), please report", errno, strerror(errno)); #else - int n; - -#ifdef HAVE_PIPE2 - n = pipe2(signal_pipe, O_CLOEXEC | O_NONBLOCK); -#else - n = pipe(signal_pipe); -#endif - - assert(!n); - if (n) + if (open_pipe(signal_pipe)) log_message(LOG_INFO, "BUG - pipe in signal_handler_init failed - %d (%s), please report", errno, strerror(errno)); - -#ifndef HAVE_PIPE2 - fcntl(signal_pipe[0], F_SETFL, O_NONBLOCK | fcntl(signal_pipe[0], F_GETFL)); - fcntl(signal_pipe[1], F_SETFL, O_NONBLOCK | fcntl(signal_pipe[1], F_GETFL)); - - fcntl(signal_pipe[0], F_SETFD, FD_CLOEXEC | fcntl(signal_pipe[0], F_GETFD)); - fcntl(signal_pipe[1], F_SETFD, FD_CLOEXEC | fcntl(signal_pipe[1], F_GETFD)); -#endif #endif } diff --git a/lib/utils.c b/lib/utils.c index b64188f36..14961f093 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -625,3 +625,27 @@ fork_exec(char **argv) return res; } #endif + +#if defined _WITH_VRRP_ || defined _WITH_BFD_ +int +open_pipe(int pipe_arr[2]) +{ + /* Open pipe */ +#ifdef HAVE_PIPE2 + if (pipe2(pipe_arr, O_CLOEXEC | O_NONBLOCK) == -1) +#else + if (pipe(pipe_arr) == -1) +#endif + return -1; + +#ifndef HAVE_PIPE2 + fcntl(pipe_arr[0], F_SETFL, O_NONBLOCK | fcntl(pipe_arr[0], F_GETFL)); + fcntl(pipe_arr[1], F_SETFL, O_NONBLOCK | fcntl(pipe_arr[1], F_GETFL)); + + fcntl(pipe_arr[0], F_SETFD, FD_CLOEXEC | fcntl(pipe_arr[0], F_GETFD)); + fcntl(pipe_arr[1], F_SETFD, FD_CLOEXEC | fcntl(pipe_arr[1], F_GETFD)); +#endif + + return 0; +} +#endif diff --git a/lib/utils.h b/lib/utils.h index b5215f287..9adfda641 100644 --- a/lib/utils.h +++ b/lib/utils.h @@ -131,5 +131,8 @@ extern void close_std_fd(void); #if !defined _HAVE_LIBIPTC_ || defined _LIBIPTC_DYNAMIC_ extern int fork_exec(char **argv); #endif +#if defined _WITH_VRRP_ || defined _WITH_BFD_ +extern int open_pipe(int [2]); +#endif #endif