From a66a9d9bbfc1ffb1f53ca7315c6c40e437ecccfb Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 31 Jan 2024 15:08:54 +0100 Subject: [PATCH] bgpd,lib: fix logging from rpki_create_socket() Fix the following crash when logging from rpki_create_socket(): > #0 raise (sig=) at ../sysdeps/unix/sysv/linux/raise.c:50 > #1 0x00007f6e21723798 in core_handler (signo=6, siginfo=0x7f6e1e502ef0, context=0x7f6e1e502dc0) at lib/sigevent.c:248 > #2 > #3 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 > #4 0x00007f6e2144e537 in __GI_abort () at abort.c:79 > #5 0x00007f6e2176348e in _zlog_assert_failed (xref=0x7f6e2180c920 <_xref.16>, extra=0x0) at lib/zlog.c:670 > #6 0x00007f6e216b1eda in rcu_read_lock () at lib/frrcu.c:294 > #7 0x00007f6e21762da8 in vzlog_notls (xref=0x0, prio=2, fmt=0x7f6e217afe50 "%s:%d: %s(): assertion (%s) failed", ap=0x7f6e1e504248) at lib/zlog.c:425 > #8 0x00007f6e217632fb in vzlogx (xref=0x0, prio=2, fmt=0x7f6e217afe50 "%s:%d: %s(): assertion (%s) failed", ap=0x7f6e1e504248) at lib/zlog.c:627 > #9 0x00007f6e217621f5 in zlog (prio=2, fmt=0x7f6e217afe50 "%s:%d: %s(): assertion (%s) failed") at lib/zlog.h:73 > #10 0x00007f6e21763596 in _zlog_assert_failed (xref=0x7f6e2180c920 <_xref.16>, extra=0x0) at lib/zlog.c:687 > #11 0x00007f6e216b1eda in rcu_read_lock () at lib/frrcu.c:294 > #12 0x00007f6e21762da8 in vzlog_notls (xref=0x7f6e21a50040 <_xref.68>, prio=4, fmt=0x7f6e21a4999f "getaddrinfo: debug", ap=0x7f6e1e504878) at lib/zlog.c:425 > #13 0x00007f6e217632fb in vzlogx (xref=0x7f6e21a50040 <_xref.68>, prio=4, fmt=0x7f6e21a4999f "getaddrinfo: debug", ap=0x7f6e1e504878) at lib/zlog.c:627 > #14 0x00007f6e21a3f774 in zlog_ref (xref=0x7f6e21a50040 <_xref.68>, fmt=0x7f6e21a4999f "getaddrinfo: debug") at ./lib/zlog.h:84 > #15 0x00007f6e21a451b2 in rpki_create_socket (_cache=0x55729149cc30) at bgpd/bgp_rpki.c:1337 > #16 0x00007f6e2120e7b7 in tr_tcp_open (tr_socket=0x5572914d1520) at rtrlib/rtrlib/transport/tcp/tcp_transport.c:111 > #17 0x00007f6e2120e212 in tr_open (socket=0x5572914b5e00) at rtrlib/rtrlib/transport/transport.c:16 > #18 0x00007f6e2120faa2 in rtr_fsm_start (rtr_socket=0x557290e17180) at rtrlib/rtrlib/rtr/rtr.c:130 > #19 0x00007f6e218b7ea7 in start_thread (arg=) at pthread_create.c:477 > #20 0x00007f6e21527a2f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 rpki_create_socket() is a hook function called from the rtrlib library. The issue arises because rtrlib initiates its own separate pthread in which it runs the hook, which does not establish an FRR RCU context. Consequently, this leads to failures in the logging mechanism that relies on RCU. Initialize a new FRR pthread context from the rtrlib pthread with a valid RCU context to allow logging from the rpki_create_socket() and dependent functions. Link: https://github.com/FRRouting/frr/issues/15260 Fixes: a951752d4a ("bgpd: create cache server socket in vrf") Signed-off-by: Louis Scalbert --- bgpd/bgp_rpki.c | 23 +++++++++++++++++++++++ lib/frr_pthread.c | 11 +++++++++-- lib/frrcu.c | 36 ++++++++++++++++++++++-------------- lib/frrcu.h | 6 ++++++ 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/bgpd/bgp_rpki.c b/bgpd/bgp_rpki.c index 94275762f669..4d485fc91dc3 100644 --- a/bgpd/bgp_rpki.c +++ b/bgpd/bgp_rpki.c @@ -1273,6 +1273,15 @@ static int add_cache(struct cache *cache) return SUCCESS; } +static void *rpki_pthread_start(void *arg) +{ + struct frr_pthread *fpt = arg; + + fpt->running = true; + + return NULL; +} + static int rpki_create_socket(void *_cache) { struct timeval prev_snd_tmout, prev_rcv_tmout, timeout; @@ -1281,6 +1290,7 @@ static int rpki_create_socket(void *_cache) struct tr_tcp_config *tcp_config; struct addrinfo *res = NULL; struct addrinfo hints = {}; + struct frr_pthread *fpt; socklen_t optlen; char *host, *port; struct vrf *vrf; @@ -1291,6 +1301,10 @@ static int rpki_create_socket(void *_cache) struct tr_ssh_config *ssh_config; char s_port[10]; #endif + struct frr_pthread_attr attr = { + .start = rpki_pthread_start, + .stop = frr_pthread_attr_default.stop, + }; if (!cache) return -1; @@ -1307,6 +1321,15 @@ static int rpki_create_socket(void *_cache) if (!CHECK_FLAG(vrf->status, VRF_ACTIVE) || vrf->vrf_id == VRF_UNKNOWN) return -1; + /* Create FRR pthread context to allow logging */ + fpt = frr_pthread_new(&attr, "rpki_create_socket", "rpki_create_socket"); + if (!fpt) + return -1; + fpt->rcu_thread = rcu_thread_new(NULL); + fpt->thread = cache->rtr_socket->thread_id; + if (frr_pthread_run(fpt, NULL) < 0) + return -1; + if (cache->type == TCP) { hints.ai_family = AF_UNSPEC; hints.ai_socktype = SOCK_STREAM; diff --git a/lib/frr_pthread.c b/lib/frr_pthread.c index 761969266ae4..9fd46f2067a0 100644 --- a/lib/frr_pthread.c +++ b/lib/frr_pthread.c @@ -163,8 +163,15 @@ int frr_pthread_run(struct frr_pthread *fpt, const pthread_attr_t *attr) frrtrace(1, frr_libfrr, frr_pthread_run, fpt->name); - fpt->rcu_thread = rcu_thread_prepare(); - ret = pthread_create(&fpt->thread, attr, frr_pthread_inner, fpt); + + if (!fpt->rcu_thread) + fpt->rcu_thread = rcu_thread_prepare(); + + if (fpt->thread) { + frr_pthread_inner(fpt); + ret = 0; + } else + ret = pthread_create(&fpt->thread, attr, frr_pthread_inner, fpt); /* Restore caller's signals */ pthread_sigmask(SIG_SETMASK, &oldsigs, NULL); diff --git a/lib/frrcu.c b/lib/frrcu.c index c7cc655e09ce..b85c525c5879 100644 --- a/lib/frrcu.c +++ b/lib/frrcu.c @@ -149,20 +149,9 @@ static struct rcu_thread *rcu_self(void) return (struct rcu_thread *)pthread_getspecific(rcu_thread_key); } -/* - * thread management (for the non-main thread) - */ -struct rcu_thread *rcu_thread_prepare(void) +struct rcu_thread *rcu_thread_new(void *arg) { - struct rcu_thread *rt, *cur; - - rcu_assert_read_locked(); - - if (!rcu_active) - rcu_start(); - - cur = rcu_self(); - assert(cur->depth); + struct rcu_thread *rt, *cur = arg; /* new thread always starts with rcu_read_lock held at depth 1, and * holding the same epoch as the parent (this makes it possible to @@ -172,13 +161,32 @@ struct rcu_thread *rcu_thread_prepare(void) rt->depth = 1; seqlock_init(&rt->rcu); - seqlock_acquire(&rt->rcu, &cur->rcu); + if (cur) + seqlock_acquire(&rt->rcu, &cur->rcu); rcu_threads_add_tail(&rcu_threads, rt); return rt; } +/* + * thread management (for the non-main thread) + */ +struct rcu_thread *rcu_thread_prepare(void) +{ + struct rcu_thread *cur; + + rcu_assert_read_locked(); + + if (!rcu_active) + rcu_start(); + + cur = rcu_self(); + assert(cur->depth); + + return rcu_thread_new(cur); +} + void rcu_thread_start(struct rcu_thread *rt) { pthread_setspecific(rcu_thread_key, rt); diff --git a/lib/frrcu.h b/lib/frrcu.h index e7a54dcbe5d4..9f07a69b5226 100644 --- a/lib/frrcu.h +++ b/lib/frrcu.h @@ -40,6 +40,12 @@ extern "C" { /* opaque */ struct rcu_thread; +/* sets up rcu thread info + * + * return value must be passed into the thread's call to rcu_thread_start() + */ +extern struct rcu_thread *rcu_thread_new(void *arg); + /* called before new thread creation, sets up rcu thread info for new thread * before it actually exits. This ensures possible RCU references are held * for thread startup.