Skip to content

ospfd: Use after free cleanup of lsa #19224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 22, 2025
Merged

ospfd: Use after free cleanup of lsa #19224

merged 1 commit into from
Jul 22, 2025

Conversation

donaldsharp
Copy link
Member 

ERROR: AddressSanitizer: heap-use-after-free on address 0x60c0001ca478 at pc 0x562dc173c5b9 bp 0x7ffe07f7a260 sp 0x7ffe07f7a258
READ of size 4 at 0x60c0001ca478 thread T0
    0 0x562dc173c5b8 in ospf_lsa_lock ospfd/ospf_lsa.c:269
    1 0x562dc17242ec in ospf_flood_delayed_lsa_ack ospfd/ospf_flood.c:139
    2 0x562dc17242ec in ospf_flood ospfd/ospf_flood.c:553
    3 0x562dc1771bba in ospf_ls_upd ospfd/ospf_packet.c:1959
    4 0x562dc1771bba in ospf_read_helper ospfd/ospf_packet.c:2926
    5 0x562dc1771bba in ospf_read ospfd/ospf_packet.c:2957
    6 0x7f615e0dba5f in event_call lib/event.c:2005
    7 0x7f615e001781 in frr_run lib/libfrr.c:1252
    8 0x562dc170b171 in main ospfd/ospf_main.c:307
    9 0x7f615da2c249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    10 0x7f615da2c304 in __libc_start_main_impl ../csu/libc-start.c:360
    11 0x562dc170a9d0 in _start (/usr/lib/frr/ospfd+0x15e9d0)

0x60c0001ca478 is located 56 bytes inside of 128-byte region [0x60c0001ca440,0x60c0001ca4c0)
freed by thread T0 here:
    0 0x7f615e4b76a8 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
    1 0x7f615e024478 in qfree lib/memory.c:136
    2 0x562dc173c976 in ospf_lsa_free ospfd/ospf_lsa.c:263
    3 0x562dc173ca7a in ospf_lsa_unlock ospfd/ospf_lsa.c:286
    4 0x562dc174eb11 in ospf_lsdb_delete_entry ospfd/ospf_lsdb.c:150
    5 0x562dc174f22e in ospf_lsdb_add ospfd/ospf_lsdb.c:173
    6 0x562dc1747fa5 in ospf_lsa_install ospfd/ospf_lsa.c:3071
    7 0x562dc17484db in ospf_summary_lsa_refresh ospfd/ospf_lsa.c:1436
    8 0x562dc174c116 in ospf_lsa_refresh ospfd/ospf_lsa.c:4050
    9 0x562dc174d236 in ospf_refresh_area_self_lsas ospfd/ospf_lsa.c:3826
    10 0x562dc17240d2 in ospf_flood ospfd/ospf_flood.c:533
    11 0x562dc1771bba in ospf_ls_upd ospfd/ospf_packet.c:1959
    12 0x562dc1771bba in ospf_read_helper ospfd/ospf_packet.c:2926
    13 0x562dc1771bba in ospf_read ospfd/ospf_packet.c:2957
    14 0x7f615e0dba5f in event_call lib/event.c:2005
    15 0x7f615e001781 in frr_run lib/libfrr.c:1252
    16 0x562dc170b171 in main ospfd/ospf_main.c:307
    17 0x7f615da2c249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

previously allocated by thread T0 here:
    0 0x7f615e4b83b7 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
    1 0x7f615e023b85 in qcalloc lib/memory.c:111
    2 0x562dc173c302 in ospf_lsa_new ospfd/ospf_lsa.c:193
    3 0x562dc173c5dc in ospf_lsa_new_and_data ospfd/ospf_lsa.c:212
    4 0x562dc1771192 in ospf_ls_upd_list_lsa ospfd/ospf_packet.c:1641
    5 0x562dc1771192 in ospf_ls_upd ospfd/ospf_packet.c:1726
    6 0x562dc1771192 in ospf_read_helper ospfd/ospf_packet.c:2926
    7 0x562dc1771192 in ospf_read ospfd/ospf_packet.c:2957
    8 0x7f615e0dba5f in event_call lib/event.c:2005
    9 0x7f615e001781 in frr_run lib/libfrr.c:1252
    10 0x562dc170b171 in main ospfd/ospf_main.c:307
    11 0x7f615da2c249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

Since we need to ack the lsa that was just freed, let's schedule the ack before the free.

@frrbot frrbot bot added the ospf label Jul 18, 2025
mjstapp
mjstapp reviewed Jul 18, 2025
View reviewed changes
ospfd/ospf_flood.c Outdated
@@ -547,11 +553,6 @@ int ospf_flood(struct ospf *ospf, struct ospf_neighbor *nbr,
}
}

/* Acknowledge the receipt of the LSA by sending a Link State
Acknowledgment packet back out the receiving interface. */
if (lsa_ack_flag)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah but ... if "new" is freed, it's still being used below here?
is it maybe a "lock" / ref problem? "new" might be unref'd by those "refresh" api calls, so it should have an extra ref until the end of this function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the lock on the hello send does that, right? Which is what I moved to earlier in the function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I meant: what if that extra ref doesn't happen - won't there still be a problem because the apis that unref are followed by code that touches "new"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can change it to lock the lsa for the function length. I assume you mean !lsa_ack_flag

@donaldsharp
ospfd: Use after free cleanup of lsa
6f6111b 
    ERROR: AddressSanitizer: heap-use-after-free on address 0x60c0001ca478 at pc 0x562dc173c5b9 bp 0x7ffe07f7a260 sp 0x7ffe07f7a258
    READ of size 4 at 0x60c0001ca478 thread T0
        0 0x562dc173c5b8 in ospf_lsa_lock ospfd/ospf_lsa.c:269
        1 0x562dc17242ec in ospf_flood_delayed_lsa_ack ospfd/ospf_flood.c:139
        2 0x562dc17242ec in ospf_flood ospfd/ospf_flood.c:553
        3 0x562dc1771bba in ospf_ls_upd ospfd/ospf_packet.c:1959
        4 0x562dc1771bba in ospf_read_helper ospfd/ospf_packet.c:2926
        5 0x562dc1771bba in ospf_read ospfd/ospf_packet.c:2957
        6 0x7f615e0dba5f in event_call lib/event.c:2005
        7 0x7f615e001781 in frr_run lib/libfrr.c:1252
        8 0x562dc170b171 in main ospfd/ospf_main.c:307
        9 0x7f615da2c249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
        10 0x7f615da2c304 in __libc_start_main_impl ../csu/libc-start.c:360
        11 0x562dc170a9d0 in _start (/usr/lib/frr/ospfd+0x15e9d0)

    0x60c0001ca478 is located 56 bytes inside of 128-byte region [0x60c0001ca440,0x60c0001ca4c0)
    freed by thread T0 here:
        0 0x7f615e4b76a8 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
        1 0x7f615e024478 in qfree lib/memory.c:136
        2 0x562dc173c976 in ospf_lsa_free ospfd/ospf_lsa.c:263
        3 0x562dc173ca7a in ospf_lsa_unlock ospfd/ospf_lsa.c:286
        4 0x562dc174eb11 in ospf_lsdb_delete_entry ospfd/ospf_lsdb.c:150
        5 0x562dc174f22e in ospf_lsdb_add ospfd/ospf_lsdb.c:173
        6 0x562dc1747fa5 in ospf_lsa_install ospfd/ospf_lsa.c:3071
        7 0x562dc17484db in ospf_summary_lsa_refresh ospfd/ospf_lsa.c:1436
        8 0x562dc174c116 in ospf_lsa_refresh ospfd/ospf_lsa.c:4050
        9 0x562dc174d236 in ospf_refresh_area_self_lsas ospfd/ospf_lsa.c:3826
        10 0x562dc17240d2 in ospf_flood ospfd/ospf_flood.c:533
        11 0x562dc1771bba in ospf_ls_upd ospfd/ospf_packet.c:1959
        12 0x562dc1771bba in ospf_read_helper ospfd/ospf_packet.c:2926
        13 0x562dc1771bba in ospf_read ospfd/ospf_packet.c:2957
        14 0x7f615e0dba5f in event_call lib/event.c:2005
        15 0x7f615e001781 in frr_run lib/libfrr.c:1252
        16 0x562dc170b171 in main ospfd/ospf_main.c:307
        17 0x7f615da2c249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

    previously allocated by thread T0 here:
        0 0x7f615e4b83b7 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
        1 0x7f615e023b85 in qcalloc lib/memory.c:111
        2 0x562dc173c302 in ospf_lsa_new ospfd/ospf_lsa.c:193
        3 0x562dc173c5dc in ospf_lsa_new_and_data ospfd/ospf_lsa.c:212
        4 0x562dc1771192 in ospf_ls_upd_list_lsa ospfd/ospf_packet.c:1641
        5 0x562dc1771192 in ospf_ls_upd ospfd/ospf_packet.c:1726
        6 0x562dc1771192 in ospf_read_helper ospfd/ospf_packet.c:2926
        7 0x562dc1771192 in ospf_read ospfd/ospf_packet.c:2957
        8 0x7f615e0dba5f in event_call lib/event.c:2005
        9 0x7f615e001781 in frr_run lib/libfrr.c:1252
        10 0x562dc170b171 in main ospfd/ospf_main.c:307
        11 0x7f615da2c249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

Since we need to ack the lsa that was just freed, let's schedule the ack
before the free.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
@donaldsharp donaldsharp force-pushed the ospf_use_after_free_double_flumple branch from 6e01bf7 to 6f6111b Compare July 22, 2025 12:47
@github-actions github-actions bot added size/XS and removed size/S labels Jul 22, 2025
mjstapp
mjstapp approved these changes Jul 22, 2025
View reviewed changes
Copy link
Contributor

@mjstapp mjstapp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I think that's the safest thing, whatever happens in between

riw777
riw777 approved these changes Jul 22, 2025
View reviewed changes
Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@Jafaral
Copy link
Member

Jafaral commented Jul 22, 2025

@Mergifyio backport stable/10.4 stable/10.3

@mergify Mergify
Copy link

mergify bot commented Jul 22, 2025 •
edited
Loading

backport stable/10.4 stable/10.3

✅ Backports have been created

@Jafaral Jafaral merged commit d5d1625 into FRRouting:master Jul 22, 2025
14 checks passed
This was referenced Jul 22, 2025
Merged
Merged
mjstapp added a commit that referenced this pull request Jul 22, 2025
@mjstapp
Merge pull request #19238 from FRRouting/mergify/bp/stable/10.4/pr-19224
5b7854a 
ospfd: Use after free cleanup of lsa (backport #19224)
mjstapp added a commit that referenced this pull request Jul 22, 2025
@mjstapp
Merge pull request #19239 from FRRouting/mergify/bp/stable/10.3/pr-19224
e499302 
ospfd: Use after free cleanup of lsa (backport #19224)
@donaldsharp donaldsharp deleted the ospf_use_after_free_double_flumple branch July 30, 2025 17:05
Jafaral added a commit that referenced this pull request Aug 2, 2025
@Jafaral
FRR Release 10.4.1
88f5c06 
Bug Fixes:

* bgpd: initialize local variable (backport #19233)
* ospfd: Use after free cleanup of lsa (backport #19224)
* vtysh: copy config from file should actually apply (backport #19242)
* Revert PR #18358: BGP evpn testing and bug fixes related to non default EVPN backbone  (backport #19241)
* topotests: improve embedded RP test reliability (backport #19240)
* lib, zebra: mark singleton nexthops inactive/active on link state changes for wecmp (backport #18947)
* bgpd: LL next-hop capabilty fixes (backport #19261)
* eigrp: validate hello packets and tlvs better (backport #19251)
* bgpd: Fix compilation error in bgpd module: Update TP_ARGS for bgp (backport #19266)
* bgpd: Ensure addpath does not withdraw selected route in some situations (backport #19210)
* bgpd: [GR] fixed selectionDeferralTimer to display select_defer_time val by #19282
* bgpd: LL next-hop capabilty fixes (round 2) (backport #19277)
* lib: compute link-state zapi message size (backport #19290)
* zebra: Fix buffer overflows found by fuzzing. (backport #19303)

Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
ton31337 added a commit to opensourcerouting/frr that referenced this pull request Aug 2, 2025
@ton31337
FRR Release 10.3.2
e50cba8 
* bgpd: correct no form commands (backport FRRouting#18911)
* bgpd: fix to show exist/non-exist-map in 'show run' properly FRRouting#18853
* redhat: make FRR RPM build to work on RedHat 10 (backport FRRouting#18920)
* build: check for libunwind.h, not unwind.h (backport FRRouting#18912)
* bgpd: use AS4B format for BGP loc-rib messages. (backport FRRouting#18936)
* bgpd: fix for the validity and the presence of prefixes in the BGP VPN table. (backport FRRouting#17370)
* bgpd: Force adj-rib-out updates if MRAI is kicked in (backport FRRouting#18959)
* zebra: Provide SID value when sending SRv6 SID release notify message (backport FRRouting#18971)
* bgpd: Fix crash when fetching statistics for bgp instance (backport FRRouting#19003)
* nhrpd: fix crash when accessing invalid memory zone (backport FRRouting#18994)
* zebra: Initialize RB tree for router tables (backport FRRouting#19049)
* zebra: fix null pointer dereference in zebra_evpn_sync_neigh_del (backport FRRouting#19054)
* zebra: fix stale NHG in kernel (backport FRRouting#18899)
* bgpd: Fix incorrect stripping of transitive extended communities (backport FRRouting#19065)
* lib: Fix no on-match goto NUM command (backport FRRouting#19108)
* bgpd: Fix extended community check for IP non-transitive type (backport FRRouting#19097)
* bgpd: Fix DEREF_OF_NULL.EX.COND in bgp_updgrp_packet (backport FRRouting#19126)
* lib: revert addition of vtysh_flush() call in vty_out() (backport FRRouting#19109)
* bgpd: Extract link bandwidth value from extcommunity before using for WCMP (backport FRRouting#19165)
* Use ipv4 class E addresses (240.0.0.0/4) as connected routes by default (backport FRRouting#18095)
* bfdd: Set bfd.LocalDiag when transitioning to AdminDown (backport FRRouting#18592)
* zebra: clean up a json object leak (backport FRRouting#19192)
* bgpd: Do not try to reuse freed route-maps (backport FRRouting#19191)
* lib: fix routemap crash (backport FRRouting#19127)
* bgpd: initialize local variable (backport FRRouting#19233)
* ospfd: Use after free cleanup of lsa (backport FRRouting#19224)
* vtysh: copy config from file should actually apply (backport FRRouting#19242)
* bgpd : Fix compilation error in bgpd module: Update TP_ARGS for bgp (backport FRRouting#19266)
* bgpd: Ensure addpath does not withdraw selected route in some situations (backport FRRouting#19210)
* lib, zebra: mark singleton nexthops inactive/active on link state changes for wecmp (backport FRRouting#18947)
* eigrp: validate hello packets and tlvs better (backport FRRouting#19251)
* bgpd: [GR] fixed selectionDeferralTimer to display select_defer_time val FRRouting#19283

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@riw777 riw777 riw777 approved these changes

@mjstapp mjstapp mjstapp approved these changes

Assignees
No one assigned
Labels
backport master ospf size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@donaldsharp @Jafaral @riw777 @mjstapp