isisd: add unit test for remove_excess_adjs() memory leak fixAdd a test that verifies remove_excess_adjs() properly frees the
isis_vertex_adj it removes from the list. The test uses FRR's
per-MTYPE allocation counter (n_alloc) to assert that:
- After remove_excess_adjs(): exactly one MTYPE_ISIS_VERTEX_ADJ
allocation is freed (not just unlinked)
- After full list cleanup: all allocations return to baseline
Without the fix in the preceding commit, the ...
isisd: fix memory leak in remove_excess_adjs()When the number of adjacencies for a vertex exceeds
ISIS_MAX_PATH_SPLITS, remove_excess_adjs() calls list_delete_node()
which frees the listnode but does NOT free the isis_vertex_adj data
payload. This leaks one MTYPE_ISIS_VERTEX_ADJ allocation per call.
In large-scale deployments, each SPF run triggers this path
accumulating leaked memory.
Valgrind trace:
definitely lost: 3,332,608 bytes i...
bfdd: cap IPv6 echo reflection to declared lengthValidate IPv6 echo payload length and reflect only bep->len
bytes instead of raw receive length to avoid oversized
reflection payloads.
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
bfdd: account for FP offset in echo length checksValidate echo packet length using bfd_offset + sizeof(*bep) so
forwarding-plane packets are not parsed past received data.
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
bfdd: fix recv errno filter logic in a few placesUse logical AND when filtering EAGAIN/EWOULDBLOCK/EINTR in
bfd_recv_ipv4_fp(), bfd_recv_ipv4(), and bfd_recv_ipv6() so
only real socket errors are logged.
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
bfdd: gate IPv6 echo reflection on known sessionsOnly reflect IPv6 echo packets when the source/local tuple maps to
an existing BFD session, preventing unauthenticated reflection of
arbitrary on-link packets.
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
bfdd: validate control packet length before session lookupCheck recv length before casting/parsing the control packet so short
or error reads cannot drive discriminator/session lookup with
uninitialized data. Keep debug visibility for short and non-positive
reads while dropping early.
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
ci: skip github ci for mergify PRsSkip this workflow when the PR actor is mergify[bot] to reduce load.
We still run NeDEF CI, so if there are issues they would still be
caught.
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
ci: upgrade GitHub actions for Node 24Bump checkout, upload-artifact, and download-artifact to
Node 24-compatible major versions so CI remains compatible
with the runner runtime migration away from Node 20.
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
isisd: fix edge condition in max_lsp_count computationFix an edge condition in the arithmetic in the max_lsp_count
api.
Signed-off-by: Mark Stapp <mjs@cisco.com>
tests: fix grpc_basic xdist collection mismatchAdd a deterministic import check for grpc and grpc_tools before the
subprocess-based --check call. When gRPC is not installed, all xdist
workers skip consistently, avoiding the collection mismatch that
causes pytest-xdist to abort with "Different tests were collected".
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
tests: add topotest for BGP I/O thread CPU spin on full input queueAdd a stress test that replicates the I/O thread spin bug fixed in commit
ed405bf22 ("bgpd: fix I/O thread spinning when peer input queue is
full").
A raw BGP speaker (bgp_sender.py) blasts 10000 UPDATE messages via
non-blocking I/O, each carrying a 15-ASN AS_PATH to increase per-route
processing cost. The total data (~740 KB) exceeds the ibuf_work ring
buffer (~96 KB), creating sustained TCP ...
tests: Slow down test_config.py to allow for processing time to happenThe code has this pattern:
a) Input some cli
b) Look for success
The test is not being graceful in that under heavy load, a) might
not have finished. Give the test system more time to get to an answer.
Please note, I am actually still seeing a honest to goodness bug in mgmtd
that this test is exposing, but the messages about the `cli is locked` and
test failing for not being given enough ti...
bgpd: Fix missing present_tlvs bit for Link ID in link NLRIWhen originating/withdrawing a Link NLRI, link_remote_id is filled in
the bgp_ls_nlri structure but BGP_LS_LINK_DESC_LINK_ID_BIT is not set
in link_desc.present_tlvs.
Fix by setting BGP_LS_LINK_DESC_LINK_ID_BIT in both bgp_ls_originate_link()
and bgp_ls_withdraw_link().
Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
bgpd: Require valid TED objects in BGP-LS originate/withdraw APIsBGP-LS node/link/prefix originate and withdraw handlers are expected to
receive valid TED objects.
Add explicit checks at the beginning of each function and return early on
invalid inputs, before any further processing.
This makes the API contract clear, avoids NULL dereferences, and keeps the
originate/withdraw paths consistent.
Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
bgpd: Include local-node ASN in link withdraw NLRIA Link NLRI contains two nodes: local node and remote node.
Per RFC 9552, each node is identified by ASN, OSPF Area ID, and
IGP Router ID.
For the remote node, `bgp_ls_withdraw_link` sets ASN, OSPF Area ID, and
IGP Router ID when generating the NLRI.
For the local node, `bgp_ls_withdraw_link` sets only OSPF Area ID and
IGP Router ID when generating the NLRI.
Add ASN for the local node as wel...
bgpd: Fix use-after-free in BGP-LS node origination`bgp_ls_originate_node()` could free `ls_attr` after `bgp_ls_populate_node_attr()`
failure, then continue and pass the freed pointer to `bgp_ls_update()`.
Fix by returning immediately after `bgp_ls_attr_free(ls_attr)` on
populate failure.
Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
lib: fix crash in thread_process_io_inner_loop on stale epoll eventWhen do_event_cancel() processes a pending cancellation at the top of
event_fetch_inner_loop(), it removes the fd from the epoll_event_hash
and calls EPOLL_CTL_DEL. However, epoll_wait() can still deliver events
that were already queued in the kernel's ready list before the
EPOLL_CTL_DEL took effect.
When thread_process_io_inner_loop() processes such a stale event, the
hash lookup returns NULL...
bgpd: Clear `registered_ls_db` before calling `ls_unregister()``bgp_ls_unregister()` only cleared `registered_ls_db` after a
successful `ls_unregister()` call. When `ls_unregister()` failed, the
flag was left as true, making `bgp_ls_is_registered()` report "still
registered". Any subsequent call to `bgp_ls_register()` would then
return early thinking registration was already in place, leaving BGP
permanently unable to receive link-state updates from zebr...
bgpd: Unintern old NLRI reference in `bgp_ls_update()``bgp_afi_node_get()` returns an existing RIB node when the prefix is
already present. In that case `dest->ls_nlri` already holds an
interned pointer. Unconditionally overwriting it with the new value
dropped the old reference without calling `bgp_ls_nlri_unintern()`,
leaking the previous allocation.
Unintern the existing pointer before installing the new one so the
refcount is kept accurate ...
bgpd: Use `bgp_node_lookup()` in `bgp_ls_withdraw()``bgp_ls_withdraw()` was calling `bgp_afi_node_get()` to locate the RIB
destination before marking the route as withdrawn. `bgp_afi_node_get()`
creates a new RIB node when none exists, so a withdraw for an NLRI that
was never installed silently created a phantom RIB entry with no path
info attached, wasting memory and polluting the table.
Replace the call with `bgp_node_lookup()`, which is a p...
bgpd: Fix `edge->destination` null deref in `bgp_ls_withdraw_link()``bgp_ls_withdraw_link()` dereferenced `edge->destination->node` to
read the remote AS number, but the guard only checked
`edge->attributes` — it did not verify that `edge->destination` or
`edge->destination->node` were non-NULL, leading to a potential null
dereference.
Add the missing null checks on `edge->destination` and
`edge->destination->node`, consistent with the guard used for the
same ...
bgpd: Demote unknown opaque message warning to debug in opaque handlerBGP registers only for `LINK_STATE_SYNC` and `LINK_STATE_UPDATE`
opaque messages, but other daemons may send opaque messages on the
same channel for their own purposes. The `default` case in
`bgp_zebra_opaque_msg_handler()` emitted a `zlog_warn()` for every
such message, which would be noisy and gives the operator nothing to
act on.
Demote to a debug log guarded by `BGP_DEBUG(zebra, ZEBRA)`.
...
bgpd: Remove duplicate log calls in `bgp_ls_withdraw_node()``bgp_ls_withdraw_node()` called `zlog_err()` immediately followed by
`flog_err()` with identical messages on two error paths. `flog_err()`
already logs to the standard output, making the `zlog_err()` calls
redundant. Remove the duplicates.
Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
bgpd: Align `bgp_ls_populate_prefix_attr()` with node/link pattern`bgp_ls_populate_prefix_attr()` used a double-pointer `**attr` and
allocated `ls_attr` internally, unlike `bgp_ls_populate_node_attr()`
and `bgp_ls_populate_link_attr()` which receive a pre-allocated
`*attr` from the caller.
The internal `!encoded` path also had a bug: `attr = NULL` set the
local variable instead of `*attr`, leaving the caller with a dangling
pointer to freed memory.
Refactor...
bgpd: Fix memory leak in `bgp_ls_originate_node()`In `bgp_ls_originate_node()`, `ls_attr` is allocated with
`bgp_ls_attr_alloc()` and populated before being passed to
`bgp_ls_update()`. If `bgp_ls_update()` fails, the function
returns -1 without freeing `ls_attr`, causing a memory leak.
Free `ls_attr` before returning on the `bgp_ls_update()` failure
path.
Signed-off-by: Carmine Scarpitta <cscarpit@cisco.com>
bgpd: Fix null dereference when `bgp->ls_info` is NULL`bgp_ls_register()` calls `bgp_ls_is_registered()` as an early-return
guard, but that function returns false when `bgp->ls_info` is NULL.
The code then proceeds to dereference `bgp->ls_info->registered_ls_db`
unconditionally, causing a crash.
Add explicit null guards at the top of `bgp_ls_register()`,
`bgp_ls_unregister()`, and `bgp_ls_cleanup()` to return early when
`bgp->ls_info` is not init...
pimd: Reject pim packets with a malformed header lengthEnsure that the header length passed in is correct.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
pimd: Fix out of bounds read in AutoRP codeThe pim_autorp.c has an out-of-bounds read in the announcement/discovery
parsing when unsupported RP entries are skipped. Fix this.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
pimd: igmpv3 never checks packet length and trusts the num-sources fieldModify the code to ensure that the packet length is good enough to allow
us to continue reading the packet instead of just trusting the number
of sources field.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Mark Stapp
Quentin Baradat
Jafar Al-Gharaibeh
Mark Stapp
Ke Zhang
Donald Sharp
Carmine Scarpitta
Manpreet Kaur