From 745a0fcbb21a6bd7330454437abe34aff0bf82c0 Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Fri, 7 Jul 2023 02:55:18 +0200 Subject: [PATCH 1/2] zebra: Abstract `dplane_ctx_route_init` to init route without copying The function `dplane_ctx_route_init` initializes a dplane route context from the route object passed as an argument. Let's abstract this function to allow initializing the dplane route context without actually copying a route object. This allows us to use this function for initializing a dplane route context when we don't have any route to copy in it. Signed-off-by: Carmine Scarpitta --- zebra/zebra_dplane.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index 6dd1a9c18d..6a685267a9 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -3257,7 +3257,7 @@ int dplane_ctx_route_init_basic(struct zebra_dplane_ctx *ctx, { int ret = EINVAL; - if (!ctx || !re) + if (!ctx) return ret; dplane_intf_extra_list_init(&ctx->u.rinfo.intf_extra_list); @@ -3265,6 +3265,13 @@ int dplane_ctx_route_init_basic(struct zebra_dplane_ctx *ctx, ctx->zd_op = op; ctx->zd_status = ZEBRA_DPLANE_REQUEST_SUCCESS; + /* This function may be called to create/init a dplane context, not + * necessarily to copy a route object. Let's return if there is no route + * object to copy. + */ + if (!re) + return AOK; + ctx->u.rinfo.zd_type = re->type; ctx->u.rinfo.zd_old_type = re->type; @@ -3296,6 +3303,8 @@ int dplane_ctx_route_init_basic(struct zebra_dplane_ctx *ctx, /* * Initialize a context block for a route update from zebra data structs. + * If the `rn` or `re` parameters are NULL, this function only initializes the + * dplane context without copying a route object into it. */ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, struct route_node *rn, struct route_entry *re) @@ -3312,9 +3321,17 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, const struct interface *ifp; struct dplane_intf_extra *if_extra; - if (!ctx || !rn || !re) + if (!ctx) return ret; + /* + * Initialize the dplane context and return, if there is no route + * object to copy + */ + if (!re || !rn) + return dplane_ctx_route_init_basic(ctx, op, NULL, NULL, NULL, + AFI_UNSPEC, SAFI_UNSPEC); + /* * Let's grab the data from the route_node * so that we can call a helper function From 7f2dec4f090500c65f54fb7ce3c2bab22eef5474 Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Fri, 7 Jul 2023 02:57:07 +0200 Subject: [PATCH 2/2] zebra: Fix crash when `dplane_fpm_nl` fails to process received routes When `dplane_fpm_nl` receives a route, it allocates memory for a dplane context and calls `netlink_route_change_read_unicast_internal` without initializing the `intf_extra_list` contained in the dplane context. If `netlink_route_change_read_unicast_internal` is not able to process the route, we call `dplane_ctx_fini` to free the dplane context. This causes a crash because `dplane_ctx_fini` attempts to access the intf_extra_list which is not initialized. To solve this issue, we can call `dplane_ctx_route_init`to initialize the dplane route context properly, just after the dplane context allocation. (gdb) bt #0 0x0000555dd5ceae80 in dplane_intf_extra_list_pop (h=0x7fae1c007e68) at ../zebra/zebra_dplane.c:427 #1 dplane_ctx_free_internal (ctx=0x7fae1c0074b0) at ../zebra/zebra_dplane.c:724 #2 0x0000555dd5cebc99 in dplane_ctx_free (pctx=0x7fae2aa88c98) at ../zebra/zebra_dplane.c:869 #3 dplane_ctx_free (pctx=0x7fae2aa88c98, pctx@entry=0x7fae2aa78c28) at ../zebra/zebra_dplane.c:855 #4 dplane_ctx_fini (pctx=pctx@entry=0x7fae2aa88c98) at ../zebra/zebra_dplane.c:890 #5 0x00007fae31e93f29 in fpm_read (t=) at ../zebra/dplane_fpm_nl.c:605 #6 0x00007fae325191dd in thread_call (thread=thread@entry=0x7fae2aa98da0) at ../lib/thread.c:2006 #7 0x00007fae324c42b8 in fpt_run (arg=0x555dd74777c0) at ../lib/frr_pthread.c:309 #8 0x00007fae32405ea7 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 #9 0x00007fae32325a2f in clone () from /lib/x86_64-linux-gnu/libc.so.6 Fixes: #13754 Signed-off-by: Carmine Scarpitta --- zebra/dplane_fpm_nl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zebra/dplane_fpm_nl.c b/zebra/dplane_fpm_nl.c index 51b1df2558..02881a6447 100644 --- a/zebra/dplane_fpm_nl.c +++ b/zebra/dplane_fpm_nl.c @@ -587,7 +587,8 @@ static void fpm_read(struct event *t) switch (hdr->nlmsg_type) { case RTM_NEWROUTE: ctx = dplane_ctx_alloc(); - dplane_ctx_set_op(ctx, DPLANE_OP_ROUTE_NOTIFY); + dplane_ctx_route_init(ctx, DPLANE_OP_ROUTE_NOTIFY, NULL, + NULL); if (netlink_route_change_read_unicast_internal( hdr, 0, false, ctx) != 1) { dplane_ctx_fini(&ctx);