From 8c8bb3568a581691403493354d4a1fad02f24c91 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 23 Jan 2024 09:16:24 +0100 Subject: [PATCH 1/5] bgpd: fix potential null pointers in rpki Fix potential NULL pointer in RPKI code. Coverity scanner issues: 1575911 1575913, 1575915, 1575917, 1575919 to 1575923, 1575925 and 1575926. Fixes: 1420189c11 ("bgpd: add support of rpki in vrf configure context") Signed-off-by: Louis Scalbert --- bgpd/bgp_rpki.c | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_rpki.c b/bgpd/bgp_rpki.c index 219cb299834e..332f3c8def47 100644 --- a/bgpd/bgp_rpki.c +++ b/bgpd/bgp_rpki.c @@ -1621,11 +1621,15 @@ DEFUN_NOSH (rpki, { struct rpki_vrf *rpki_vrf; char *vrfname = NULL; + struct vrf *vrf; if (vty->node == CONFIG_NODE) vty->node = RPKI_NODE; else { - struct vrf *vrf = VTY_GET_CONTEXT(vrf); + vrf = VTY_GET_CONTEXT(vrf); + + if (!vrf) + return CMD_WARNING; vty->node = RPKI_VRF_NODE; if (vrf->vrf_id != VRF_DEFAULT) @@ -1732,6 +1736,9 @@ DEFPY (rpki_polling_period, else rpki_vrf = VTY_GET_CONTEXT(rpki_vrf); + if (!rpki_vrf) + return CMD_WARNING_CONFIG_FAILED; + rpki_vrf->polling_period = pp; return CMD_SUCCESS; } @@ -1751,6 +1758,9 @@ DEFUN (no_rpki_polling_period, else rpki_vrf = VTY_GET_CONTEXT(rpki_vrf); + if (!rpki_vrf) + return CMD_WARNING_CONFIG_FAILED; + rpki_vrf->polling_period = POLLING_PERIOD_DEFAULT; return CMD_SUCCESS; } @@ -1769,6 +1779,9 @@ DEFPY (rpki_expire_interval, else rpki_vrf = VTY_GET_CONTEXT(rpki_vrf); + if (!rpki_vrf) + return CMD_WARNING_CONFIG_FAILED; + if ((unsigned int)tmp >= rpki_vrf->polling_period) { rpki_vrf->expire_interval = tmp; return CMD_SUCCESS; @@ -1793,6 +1806,9 @@ DEFUN (no_rpki_expire_interval, else rpki_vrf = VTY_GET_CONTEXT(rpki_vrf); + if (!rpki_vrf) + return CMD_WARNING_CONFIG_FAILED; + rpki_vrf->expire_interval = rpki_vrf->polling_period * 2; return CMD_SUCCESS; } @@ -1811,6 +1827,9 @@ DEFPY (rpki_retry_interval, else rpki_vrf = VTY_GET_CONTEXT(rpki_vrf); + if (!rpki_vrf) + return CMD_WARNING_CONFIG_FAILED; + rpki_vrf->retry_interval = tmp; return CMD_SUCCESS; } @@ -1830,6 +1849,9 @@ DEFUN (no_rpki_retry_interval, else rpki_vrf = VTY_GET_CONTEXT(rpki_vrf); + if (!rpki_vrf) + return CMD_WARNING_CONFIG_FAILED; + rpki_vrf->retry_interval = RETRY_INTERVAL_DEFAULT; return CMD_SUCCESS; } @@ -1861,6 +1883,9 @@ DEFPY(rpki_cache, rpki_cache_cmd, else rpki_vrf = VTY_GET_CONTEXT(rpki_vrf); + if (!rpki_vrf) + return CMD_WARNING_CONFIG_FAILED; + if (!rpki_vrf || !rpki_vrf->cache_list) return CMD_WARNING; @@ -1930,6 +1955,9 @@ DEFPY (no_rpki_cache, else rpki_vrf = VTY_GET_CONTEXT(rpki_vrf); + if (!rpki_vrf) + return CMD_WARNING_CONFIG_FAILED; + cache_list = rpki_vrf->cache_list; cache_p = find_cache(preference, cache_list); if (!rpki_vrf || !cache_p) { @@ -2422,6 +2450,10 @@ static int config_on_exit(struct vty *vty) rpki_vrf = VTY_GET_CONTEXT_SUB(rpki_vrf); else rpki_vrf = VTY_GET_CONTEXT(rpki_vrf); + + if (!rpki_vrf) + return CMD_WARNING_CONFIG_FAILED; + reset(false, rpki_vrf); return 1; } @@ -2454,6 +2486,10 @@ DEFPY (rpki_reset_config_mode, rpki_vrf = VTY_GET_CONTEXT_SUB(rpki_vrf); else rpki_vrf = VTY_GET_CONTEXT(rpki_vrf); + + if (!rpki_vrf) + return CMD_WARNING_CONFIG_FAILED; + return reset(true, rpki_vrf) == SUCCESS ? CMD_SUCCESS : CMD_WARNING; } From 39c8c97d41616c6fb0abf92c48c24b8efd2a7ffe Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 23 Jan 2024 09:29:26 +0100 Subject: [PATCH 2/5] bgpd: fix deference before check in rpki_create_socket Fix deference before check coverity scanner issue 1575918 in rpki_create_socket() Fixes: a951752d4a ("bgpd: create cache server socket in vrf") Signed-off-by: Louis Scalbert --- bgpd/bgp_rpki.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_rpki.c b/bgpd/bgp_rpki.c index 332f3c8def47..de5476173df5 100644 --- a/bgpd/bgp_rpki.c +++ b/bgpd/bgp_rpki.c @@ -1276,7 +1276,7 @@ static int rpki_create_socket(void *_cache) { struct timeval prev_snd_tmout, prev_rcv_tmout, timeout; struct cache *cache = (struct cache *)_cache; - struct rpki_vrf *rpki_vrf = cache->rpki_vrf; + struct rpki_vrf *rpki_vrf; struct tr_tcp_config *tcp_config; struct addrinfo *res = NULL; struct addrinfo hints = {}; @@ -1294,6 +1294,8 @@ static int rpki_create_socket(void *_cache) if (!cache) return -1; + rpki_vrf = cache->rpki_vrf; + if (rpki_vrf->vrfname == NULL) vrf = vrf_lookup_by_id(VRF_DEFAULT); else From b28fd4e527e28bea98bd1e396f5c617051561d35 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 23 Jan 2024 11:52:59 +0100 Subject: [PATCH 3/5] bgpd: fix res validity in rpki_create_socket Fix coverity scanner issue 1575912 where res pointer is supposed to valid in: > socket = vrf_socket(res->ai_family, ...) but is checked for validity a few lines later. Note that vrf_getaddrinfo returns an error code if getaddrinfo() fails to allocate res and in this case, rpki_create_socket() returns. Fixes: a951752 ("bgpd: create cache server socket in vrf") Signed-off-by: Louis Scalbert --- bgpd/bgp_rpki.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/bgpd/bgp_rpki.c b/bgpd/bgp_rpki.c index de5476173df5..2c716a0e28ba 100644 --- a/bgpd/bgp_rpki.c +++ b/bgpd/bgp_rpki.c @@ -25,6 +25,7 @@ #include "memory.h" #include "frrevent.h" #include "filter.h" +#include "lib_errors.h" #include "bgpd/bgpd.h" #include "bgpd/bgp_table.h" #include "bgp_advertise.h" @@ -1332,8 +1333,11 @@ static int rpki_create_socket(void *_cache) frr_with_privs (&bgpd_privs) { ret = vrf_getaddrinfo(host, port, &hints, &res, vrf->vrf_id); } - if (ret != 0) + if (ret != 0) { + flog_err_sys(EC_LIB_SOCKET, "getaddrinfo: %s", + gai_strerror(ret)); return -1; + } frr_with_privs (&bgpd_privs) { socket = vrf_socket(res->ai_family, res->ai_socktype, @@ -1354,15 +1358,13 @@ static int rpki_create_socket(void *_cache) setsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, &timeout, sizeof(timeout)); if (connect(socket, res->ai_addr, res->ai_addrlen) == -1) { - if (res) - freeaddrinfo(res); + freeaddrinfo(res); close(socket); pthread_setcancelstate(cancel_state, NULL); return -1; } - if (res) - freeaddrinfo(res); + freeaddrinfo(res); pthread_setcancelstate(cancel_state, NULL); setsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, &prev_rcv_tmout, From a1d4769eca276ef23a55cb486b11aeb6e9790ad1 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 23 Jan 2024 11:52:04 +0100 Subject: [PATCH 4/5] bgpd: fix memory leak in rpki_create_socket Fix memory leak in rpki_create_socket. Coverity scanner issue 1575914. Fixes: a951752 ("bgpd: create cache server socket in vrf") Signed-off-by: Louis Scalbert --- bgpd/bgp_rpki.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_rpki.c b/bgpd/bgp_rpki.c index 2c716a0e28ba..65f4f1e08975 100644 --- a/bgpd/bgp_rpki.c +++ b/bgpd/bgp_rpki.c @@ -1343,8 +1343,10 @@ static int rpki_create_socket(void *_cache) socket = vrf_socket(res->ai_family, res->ai_socktype, res->ai_protocol, vrf->vrf_id, NULL); } - if (socket < 0) + if (socket < 0) { + freeaddrinfo(res); return -1; + } pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &cancel_state); timeout.tv_sec = 30; From efdb5f144cf9c288a172498e5fa79531efcdd767 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 23 Jan 2024 09:51:15 +0100 Subject: [PATCH 5/5] bgpd: check sockopt returns in rpki_create_socket Check (g|s)etsockopt returns in rpki_create_socket(). Coverity scanner issues 1575916 and 1575924. Fixes: a951752d4a ("bgpd: create cache server socket in vrf") Signed-off-by: Louis Scalbert --- bgpd/bgp_rpki.c | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/bgpd/bgp_rpki.c b/bgpd/bgp_rpki.c index 65f4f1e08975..9604578d9fd2 100644 --- a/bgpd/bgp_rpki.c +++ b/bgpd/bgp_rpki.c @@ -1353,11 +1353,27 @@ static int rpki_create_socket(void *_cache) timeout.tv_usec = 0; optlen = sizeof(prev_rcv_tmout); - getsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, &prev_rcv_tmout, &optlen); - getsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, &prev_snd_tmout, &optlen); - - setsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout)); - setsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, &timeout, sizeof(timeout)); + ret = getsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, &prev_rcv_tmout, + &optlen); + if (ret < 0) + zlog_warn("%s: failed to getsockopt SO_RCVTIMEO for socket %d", + __func__, socket); + ret = getsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, &prev_snd_tmout, + &optlen); + if (ret < 0) + zlog_warn("%s: failed to getsockopt SO_SNDTIMEO for socket %d", + __func__, socket); + ret = setsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, &timeout, + sizeof(timeout)); + if (ret < 0) + zlog_warn("%s: failed to setsockopt SO_RCVTIMEO for socket %d", + __func__, socket); + + ret = setsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, &timeout, + sizeof(timeout)); + if (ret < 0) + zlog_warn("%s: failed to setsockopt SO_SNDTIMEO for socket %d", + __func__, socket); if (connect(socket, res->ai_addr, res->ai_addrlen) == -1) { freeaddrinfo(res); @@ -1369,10 +1385,17 @@ static int rpki_create_socket(void *_cache) freeaddrinfo(res); pthread_setcancelstate(cancel_state, NULL); - setsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, &prev_rcv_tmout, - sizeof(prev_rcv_tmout)); - setsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, &prev_snd_tmout, - sizeof(prev_snd_tmout)); + ret = setsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, &prev_rcv_tmout, + sizeof(prev_rcv_tmout)); + if (ret < 0) + zlog_warn("%s: failed to setsockopt SO_RCVTIMEO for socket %d", + __func__, socket); + + ret = setsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, &prev_snd_tmout, + sizeof(prev_snd_tmout)); + if (ret < 0) + zlog_warn("%s: failed to setsockopt SO_SNDTIMEO for socket %d", + __func__, socket); return socket; }