pimd: fix multicast boundary list lifetime and ACL evaluation

[Jafaral]

• Fix dangling boundary list pointers by storing names and looking them up at use time
• Preserve ACL first-match order for mixed cisco/standard entries
• Clean up duplicate IGMPv3 debug output
• Add topotest coverage for list deletion and mixed ACL ordering

[greptile-apps]

Greptile Summary

This PR fixes a use-after-free hazard in pimd where pim_interface stored raw struct prefix_list * / struct access_list * pointers that became dangling when the operator deleted the referenced list. It also fixes first-match ordering for mixed standard/extended ACL entries in pim_access_list_apply, removes a duplicate type field from an IGMPv3 debug log line, and adds topotest coverage for both scenarios.

• Dangling-pointer fix: boundary_oil_plist and boundary_acl are now char * names allocated with XSTRDUP/XFREE; pim_is_group_filtered performs a lazy prefix_list_lookup/access_list_lookup at evaluation time, gracefully skipping filtering when the list no longer exists.
• ACL first-match ordering: pim_access_list_apply previously ran a cisco-only pass then fell back to access_list_apply, which re-iterated the entire list and would match cisco extended entries using only the group address (ignoring source). The new single-pass loop with the local pim_zebra_filter_match helper correctly evaluates cisco and standard entries in declaration order.

Confidence Score: 5/5

Safe to merge; the changes are well-scoped, all boundary field consumers are updated consistently, and the new test covers both the crash-safety and the ordering correctness goals.

All seven changed files are internally consistent: the type change in pim_iface.h is reflected everywhere the fields are read or written, memory management follows the established XSTRDUP/XFREE pattern used by sibling fields, and the refactored pim_access_list_apply is logically equivalent to filter_match_zebra in lib/filter.c. No call sites were overlooked.

No files require special attention; pimd/pim_util.c carries the most logic change but the single-pass rewrite is straightforward and correct.

Important Files Changed

Filename	Overview
pimd/pim_iface.h	Changed boundary_oil_plist and boundary_acl fields from raw pointers to heap-owned char *, eliminating dangling-pointer exposure when the referenced list is deleted.
pimd/pim_iface.c	Added XFREE calls for the two new string fields in pim_if_delete, matching the memory management pattern used by the existing nbr_plist and allow_rp_plist fields.
pimd/pim_nb_config.c	Replaced direct pointer assignment with XSTRDUP/XFREE of the list name in both modify and destroy callbacks; consistent with the existing pattern for nbr_plist and allow_rp_plist.
pimd/pim_util.c	Refactored pim_is_group_filtered to perform lazy name-based lookup; rewrote pim_access_list_apply with a new pim_zebra_filter_match helper so mixed cisco/standard ACL entries are evaluated in a single first-match pass.
pimd/pim_igmpv3.c	Updated debug log references from .name member to string pointer directly; removed the duplicate type=%d field from the IGMPv3 report record debug line.
pimd/pim_vty.c	Updated pim_config_write to print boundary names directly from the char * fields instead of dereferencing the old struct pointer's .name member.
tests/topotests/pim_boundary_acl/test_pim_boundary_acl.py	Adds verify_igmp_source and verify_router_running helpers plus a new test covering pimd stability after prefix-list/ACL deletion and correct first-match behavior for mixed standard+extended ACL entries.

Sequence Diagram

sequenceDiagram
    participant Admin
    participant NB as NB Config (pim_nb_config.c)
    participant Iface as pim_interface
    participant Util as pim_is_group_filtered

    Note over Admin,Util: Old behavior - dangling pointer on list deletion
    Admin->>NB: ip multicast boundary pim-acl
    NB->>Iface: "boundary_acl = access_list_lookup(pim-acl)"
    Admin->>Admin: no access-list pim-acl (list freed)
    Util->>Iface: "read boundary_acl <- dangling pointer / crash"

    Note over Admin,Util: New behavior - name stored, lazy lookup
    Admin->>NB: ip multicast boundary pim-acl
    NB->>Iface: "boundary_acl = XSTRDUP(pim-acl)"
    Admin->>Admin: no access-list pim-acl (list freed, name string intact)
    Util->>Util: "acl = access_list_lookup(boundary_acl)"
    alt ACL still exists
        Util->>Util: apply ACL filter
    else ACL deleted
        Util->>Util: skip filtering (pass traffic)
    end

Loading

_{Reviews (2): Last reviewed commit: "tests: cover boundary list deletion and ..." | Re-trigger Greptile}

[Jafaral]

@greptile review

[donaldsharp]

Why are we not hooked into pim_prefix_list_update for prefix list changes for the interfaces prefix lists? Looking up the prefix list when an operator can have 10's of thousands of them does not seem efficient or correct. Every other place I am aware of stores both the name and the pointer. When the prefix list is changed we get the callback into pim_prefix_list_update which should update the plist pointer as needed.

[Jafaral]

Sure, I will fix that.

[donaldsharp]

This seems like a separate bug fix of some sort here, that is not related to what the commit comment talks about at all.

[Jafaral]

Correct, this bug is separate but got exposed by my other fixes. Will split it in its own commit.

[donaldsharp]

Why wouldn't this check actually belong in prefix_match? It seems like an odd thing to not be checking in that function?

[Jafaral]

We can add it to prefix_match. However most users of  prefix_match  use it in a same-family context and don't need the extra check. There are a few places that add a family check explicitly. It is good to refactor those and add a common function prefix_match_with_familiy to expose the new behavior in the library. I will do that in a separate PR to avoid polluting this bug fix PR.

[riw777]

looks good

[Jafaral]

@Mergifyio backport stable/10.6 stable/10.5

[mergify]

backport stable/10.6 stable/10.5

✅ Backports have been created

Details

• #22178 pimd: fix multicast boundary list lifetime and ACL evaluation (backport #22122) has been created for branch stable/10.6 but encountered conflicts

Cherry-pick of 8db6c18 has failed:

On branch mergify/bp/stable/10.6/pr-22122
Your branch is up to date with 'origin/stable/10.6'.

You are currently cherry-picking commit 8db6c186c.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   pimd/pim6_main.c
	modified:   pimd/pim_iface.h
	modified:   pimd/pim_main.c
	modified:   pimd/pim_nb_config.c
	modified:   pimd/pim_util.c
	modified:   pimd/pim_vty.c
	modified:   pimd/pimd.c

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   pimd/pim_iface.c
	both modified:   pimd/pim_igmpv3.c

Cherry-pick of d4e324c has failed:

On branch mergify/bp/stable/10.6/pr-22122
Your branch is ahead of 'origin/stable/10.6' by 1 commit.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit d4e324c22.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   pimd/pim_igmpv3.c

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

• #22179 pimd: fix multicast boundary list lifetime and ACL evaluation (backport #22122) has been created for branch stable/10.5 but encountered conflicts

Cherry-pick of 8db6c18 has failed:

On branch mergify/bp/stable/10.5/pr-22122
Your branch is up to date with 'origin/stable/10.5'.

You are currently cherry-picking commit 8db6c186c.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   pimd/pim_iface.h
	modified:   pimd/pim_nb_config.c
	modified:   pimd/pim_util.c
	modified:   pimd/pim_vty.c

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   pimd/pim6_main.c
	both modified:   pimd/pim_iface.c
	both modified:   pimd/pim_igmpv3.c
	both modified:   pimd/pim_main.c
	both modified:   pimd/pimd.c

Cherry-pick of d4e324c has failed:

On branch mergify/bp/stable/10.5/pr-22122
Your branch is ahead of 'origin/stable/10.5' by 1 commit.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit d4e324c22.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   pimd/pim_igmpv3.c

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of 0d5338e has failed:

On branch mergify/bp/stable/10.5/pr-22122
Your branch is ahead of 'origin/stable/10.5' by 2 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit 0d5338e91.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   pimd/pim_util.c

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally