Skip to content

Commit ddf3067

Browse files
rjarrymaxime-leroy
authored andcommitted
port: unplug from graph before destroy
The port_rx node stores a direct pointer to its associated struct iface in its context (ctx->iface). This pointer is set during graph initialization and is used directly to access interface data without going through iface_from_id(). When deleting a port interface, the previous sequence was: 1. gr_event_push(IFACE_PRE_REMOVE) 2. cleanup (status down, vrf decref, nexthop cleanup) 3. ifaces[iface->id] = NULL 4. rte_rcu_qsbr_synchronize() 5. gr_event_push(IFACE_REMOVE) 6. rte_free(iface) This was not sufficient to prevent use-after-free. Even after setting ifaces[id] to NULL and waiting for RCU sync, the port_rx node would continue running and receiving packets while holding a stale pointer to the freed iface memory. This was detected by libasan: + grcli interface del p0 ... DEBUG: GROUT: iface_event: iface event [0xacdc0003] PRE_REMOVE triggered for iface p0. ... DEBUG: GROUT: iface_event: iface event [0xacdc0007] STATUS_DOWN triggered for iface p0. DEBUG: GROUT: iface_event: iface event [0xacdc0004] REMOVE triggered for iface p0. ... NOTICE: GROUT: [rx p0] 02:e3:ad:74:50:9b > 01:80:c2:00:00:02 / LACP active fast aggreg sync collect distrib, (pkt_len=124) ... INFO: GROUT: iface_port_fini: port 0 destroyed ================================================================= ==72683==ERROR: AddressSanitizer: heap-use-after-free ... READ of size 1 at 0x7f9764309b02 thread T0 lacp_input_cb ../modules/infra/control/lacp.c:42 control_queue_poll ../modules/infra/control/control_queue.c:52 event_base_loop (/lib/x86_64-linux-gnu/libevent_core-2.1.so.7+0x20fae) main ../main/main.c:326 Fix this by subscribing to GR_EVENT_IFACE_PRE_REMOVE and calling port_unplug() which disables the port queue mappings and reloads all worker graphs, effectively removing the port_rx nodes for that port. Fixes: 587d5e4 ("rx,tx: use one node per port queue") Signed-off-by: Robin Jarry <rjarry@redhat.com> Reviewed-by: Anthony Harivel <aharivel@redhat.com> Reviewed-by: Maxime Leroy <maxime@leroys.fr>
1 parent 393b14b commit ddf3067

File tree

3 files changed

+17
-0
lines changed

3 files changed

+17
-0
lines changed

modules/infra/control/port.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,22 @@ static struct gr_module port_module = {
815815
.fini = port_fini,
816816
};
817817

818+
static void port_unplug_cb(uint32_t /*event*/, const void *obj) {
819+
const struct iface *iface = obj;
820+
if (iface->type != GR_IFACE_TYPE_PORT)
821+
return;
822+
if (port_unplug(iface_info_port(iface)) < 0)
823+
LOG(WARNING, "port_unplug(%s): %s", iface->name, strerror(errno));
824+
}
825+
826+
static struct gr_event_subscription port_subscription = {
827+
.callback = port_unplug_cb,
828+
.ev_count = 1,
829+
.ev_types = {GR_EVENT_IFACE_PRE_REMOVE},
830+
};
831+
818832
RTE_INIT(port_constructor) {
819833
iface_type_register(&iface_type_port);
820834
gr_register_module(&port_module);
835+
gr_event_subscribe(&port_subscription);
821836
}

modules/infra/control/port_test.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ void gr_register_api_handler(struct gr_api_handler *) { }
2222
void gr_register_module(struct gr_module *) { }
2323
void iface_type_register(struct iface_type *) { }
2424
void gr_event_push(uint32_t, const void *) { }
25+
void gr_event_subscribe(struct gr_event_subscription *) { }
2526
mock_func(struct rte_mempool *, gr_pktmbuf_pool_get(int8_t, uint32_t));
2627
void gr_pktmbuf_pool_release(struct rte_mempool *, uint32_t) { }
2728
struct rte_rcu_qsbr *gr_datapath_rcu(void) {

modules/infra/control/worker_test.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ int gr_rte_log_type;
3838
struct gr_config gr_config;
3939
void gr_register_api_handler(struct gr_api_handler *) { }
4040
void gr_register_module(struct gr_module *) { }
41+
void gr_event_subscribe(struct gr_event_subscription *) { }
4142
void iface_type_register(struct iface_type *) { }
4243
void gr_metrics_ctx_init(struct gr_metrics_ctx *, struct gr_metrics_writer *, ...) { }
4344
void gr_metrics_labels_add(struct gr_metrics_ctx *, ...) { }

0 commit comments

Comments
 (0)