From 04f04ca93da9d1623a5952ec2b028ef3f2caf2f5 Mon Sep 17 00:00:00 2001 From: harini Date: Tue, 30 Jun 2026 07:02:51 -0700 Subject: [PATCH 1/2] pimd: clean stale upstream NHT tracking on RP delete Issue: Faced a crash in below sequence. pim_upstream_mroute_iif_update+0x16 -> pimd/pim_mroute.c:1256 pimd(+0x78a94) -> pimd/pim_nht.c pim_nexthop_update+0x587 -> pimd/pim_nht.c Rootcause: pim_rpf_update() tracks upstreams in pnc->upstream_hash keyed by up->upstream_addr. Some RP delete/BSM cleanup paths clear RPF state and then mutate up->upstream_addr to PIMADDR_ANY without first removing the upstream from the old RP nexthop cache bucket. If the upstream was previously tracked under the old RP address, a later Zebra NHT update for that address can still walk the stale upstream entry. Fix: Add pim_nht_delete_tracked_upstream() and call it from RP delete and BSM cleanup paths to remove the upstream from the old NHT bucket before up->upstream_addr is changed to PIMADDR_ANY. This prevents the old pnc->upstream_hash from retaining a stale upstream pointer. Signed-off-by: harini --- pimd/pim_bsm.c | 1 + pimd/pim_nht.c | 33 +++++++++++++++++++++++++++++++++ pimd/pim_nht.h | 4 ++++ pimd/pim_rp.c | 2 ++ 4 files changed, 40 insertions(+) diff --git a/pimd/pim_bsm.c b/pimd/pim_bsm.c index cf17036fa379..b7deb8dfe042 100644 --- a/pimd/pim_bsm.c +++ b/pimd/pim_bsm.c @@ -811,6 +811,7 @@ void pim_bsm_clear(struct pim_instance *pim) /* RP not found for the group grp */ if (!trp_info || pim_rpf_addr_is_inaddr_any(&trp_info->rp)) { + pim_nht_delete_tracked_upstream(pim, up->upstream_addr, up); pim_upstream_rpf_clear(pim, up); pim_rp_set_upstream_addr(pim, &up->upstream_addr, up->sg.src, up->sg.grp); diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c index 0cb690397419..81db0a200d50 100644 --- a/pimd/pim_nht.c +++ b/pimd/pim_nht.c @@ -664,6 +664,39 @@ static void pim_nht_drop_maybe(struct pim_instance *pim, struct pim_nexthop_cach } } +void pim_nht_delete_tracked_upstream(struct pim_instance *pim, pim_addr addr, + struct pim_upstream *up) +{ + struct pim_nexthop_cache *pnc = NULL; + struct pim_nexthop_cache lookup; + + if (!up || pim_addr_is_any(addr)) + return; + + lookup.addr = addr; + pnc = hash_lookup(pim->nht_hash, &lookup); + if (!pnc) { + if (PIM_DEBUG_PIM_NHT) + zlog_debug("%s: NHT %pPA(%s) not found for upstream %s; nothing to release", + __func__, &addr, pim->vrf->name, up->sg_str); + return; + } + + /* Remove the upstream from its old NHT bucket before upstream_addr + * is changed to ANY. + */ + if (hash_release(pnc->upstream_hash, up)) { + if (PIM_DEBUG_PIM_NHT) + zlog_debug("%s: released upstream %s from NHT %pPA(%s); remaining upstream count:%ld", + __func__, up->sg_str, &addr, pim->vrf->name, + pnc->upstream_hash->count); + pim_nht_drop_maybe(pim, pnc); + } else if (PIM_DEBUG_PIM_NHT) { + zlog_debug("%s: upstream %s not in NHT %pPA(%s) bucket; redundant cleanup", + __func__, up->sg_str, &addr, pim->vrf->name); + } +} + void pim_nht_delete_tracked(struct pim_instance *pim, pim_addr addr, struct pim_upstream *up, struct rp_info *rp) { diff --git a/pimd/pim_nht.h b/pimd/pim_nht.h index 2514f5f02e7f..a0cfe515d7dc 100644 --- a/pimd/pim_nht.h +++ b/pimd/pim_nht.h @@ -103,6 +103,10 @@ bool pim_nht_candrp_add(struct pim_instance *pim, pim_addr addr); void pim_nht_delete_tracked(struct pim_instance *pim, pim_addr addr, struct pim_upstream *up, struct rp_info *rp); +/* Delete a tracked upstream from the NHT entry for addr */ +void pim_nht_delete_tracked_upstream(struct pim_instance *pim, pim_addr addr, + struct pim_upstream *up); + /* Delete a tracked addr and decrement BSR count, if no-one else is interested, stop tracking */ void pim_nht_bsr_del(struct pim_instance *pim, pim_addr bsr_addr); diff --git a/pimd/pim_rp.c b/pimd/pim_rp.c index c3b422f5f9c9..025bbeb97790 100644 --- a/pimd/pim_rp.c +++ b/pimd/pim_rp.c @@ -794,6 +794,7 @@ int pim_rp_del(struct pim_instance *pim, pim_addr rp_addr, struct prefix group, pim_addr_to_prefix(&grp, up->sg.grp); trp_info = pim_rp_find_match_group(pim, &grp); if (trp_info == rp_all) { + pim_nht_delete_tracked_upstream(pim, rpf_addr, up); pim_upstream_rpf_clear(pim, up); up->upstream_addr = PIMADDR_ANY; } @@ -844,6 +845,7 @@ int pim_rp_del(struct pim_instance *pim, pim_addr rp_addr, struct prefix group, /* RP not found for the group grp */ if (!trp_info || pim_rpf_addr_is_inaddr_any(&trp_info->rp)) { + pim_nht_delete_tracked_upstream(pim, rpf_addr, up); pim_upstream_rpf_clear(pim, up); pim_rp_set_upstream_addr( pim, &up->upstream_addr, up->sg.src, From 18aba5c12665b2155f805b261b5c810fefb424a4 Mon Sep 17 00:00:00 2001 From: harini Date: Tue, 30 Jun 2026 07:03:37 -0700 Subject: [PATCH 2/2] pimd: guard NULL channel_oil in pim_upstream_mroute_iif_update callers pim_upstream_mroute_iif_update() dereferences c_oil via oil_incoming_vif() and pim_upstream_get_mroute_iif() with no NULL check. Some upstream cleanup paths can reach these callers after channel_oil has been freed, producing a NULL dereference. Add a defensive NULL check at the top of pim_upstream_mroute_iif_update(), and skip the call in pim_upstream_rpf_clear() and the PIM_RPF_CHANGED branch of pim_update_upstream_nh_helper() when up->channel_oil is NULL. Signed-off-by: harini --- pimd/pim_mroute.c | 3 +++ pimd/pim_nht.c | 2 +- pimd/pim_rpf.c | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pimd/pim_mroute.c b/pimd/pim_mroute.c index 637a613a22f2..af88a4dba092 100644 --- a/pimd/pim_mroute.c +++ b/pimd/pim_mroute.c @@ -1659,6 +1659,9 @@ int pim_upstream_mroute_iif_update(struct channel_oil *c_oil, const char *name) vifi_t iif; char buf[1000]; + if (!c_oil) + return 0; + iif = pim_upstream_get_mroute_iif(c_oil, name); if (*oil_incoming_vif(c_oil) == iif) { /* no change */ diff --git a/pimd/pim_nht.c b/pimd/pim_nht.c index 81db0a200d50..eaa63ef57be4 100644 --- a/pimd/pim_nht.c +++ b/pimd/pim_nht.c @@ -992,7 +992,7 @@ static int pim_update_upstream_nh_helper(struct hash_bucket *bucket, void *arg) * RPF nbr is now unreachable the MFC has already been updated * by pim_rpf_clear */ - if (rpf_result == PIM_RPF_CHANGED) + if (rpf_result == PIM_RPF_CHANGED && up->channel_oil) pim_upstream_mroute_iif_update(up->channel_oil, __func__); if (rpf_result == PIM_RPF_CHANGED || diff --git a/pimd/pim_rpf.c b/pimd/pim_rpf.c index 049dc6f85415..a80bafc4d753 100644 --- a/pimd/pim_rpf.c +++ b/pimd/pim_rpf.c @@ -212,7 +212,8 @@ void pim_upstream_rpf_clear(struct pim_instance *pim, up->rpf.source_nexthop.mrib_route_metric = router->infinite_assert_metric.route_metric; up->rpf.rpf_addr = PIMADDR_ANY; - pim_upstream_mroute_iif_update(up->channel_oil, __func__); + if (up->channel_oil) + pim_upstream_mroute_iif_update(up->channel_oil, __func__); } }