bgpd: Fix wrong union member access in bgp_ls_nlri_display()

[cscarpitta]

BGP-LS NLRIs carry different data depending on their type (Node, Link, or Prefix). Internally the data is stored in a C union, where only the member matching the active NLRI type is valid to read.

bgp_ls_nlri_display() always reads the protocol_id and identifier fields through the node union member, even when the NLRI is actually a Link or Prefix. Reading the wrong union member is undefined behavior in C; it works by coincidence because the fields happen to sit at the same memory offset in all three variants today, but would silently break if the structs were ever reordered or padded differently.

Fix by adding an upfront switch on nlri->nlri_type that reads protocol_id and identifier from whichever union member is actually active, storing them in local variables used for the rest of the function. This matches the pattern already used in the same function for the local_node field.

[greptile-apps]

Greptile Summary

This PR fixes undefined behavior in bgp_ls_nlri_display() in bgpd/bgp_ls_nlri.c, where protocol_id and identifier were always read through the node union member regardless of the active NLRI type (Node, Link, or Prefix). While this happened to work in practice due to identical field offsets in all three struct variants, it was technically UB in C and fragile to future struct layout changes.

Key changes:

• Introduces local protocol_id and identifier variables (initialized to safe defaults: BGP_LS_PROTO_RESERVED / 0)
• Adds an upfront switch on nlri->nlri_type that reads both fields from the correct union member (node, link, or prefix) before they are used
• Replaces the two direct wrong-union-member reads in the subsequent protocol-string switch and vty_out call with references to the local variables
• Covers all four active NLRI type cases plus the RESERVED no-op case, consistent with every other switch in the same function

Confidence Score: 5/5

Safe to merge — the change is a correct, well-scoped UB fix with no functional regression risk

The fix is minimal, targets only the display function, covers all union/enum cases exhaustively, uses safe defaults for the reserved case, and matches the existing pattern used for local_node in the same function. All three union structs share the same protocol_id/identifier layout so the fix produces identical runtime output while eliminating the undefined behavior.

No files require special attention

Important Files Changed

Filename	Overview
bgpd/bgp_ls_nlri.c	Adds a correct upfront switch to extract protocol_id and identifier from the active union member, fixing UB where these fields were always read through the node member regardless of NLRI type; all enum cases handled, defaults are safe

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[bgp_ls_nlri_display called] --> B{nlri == NULL?}
    B -- yes --> C[return]
    B -- no --> D{switch nlri_type\nExtract protocol_id & identifier}
    D -- NODE --> E[read from nlri_data.node]
    D -- LINK --> F[read from nlri_data.link]
    D -- IPV4/IPV6 PREFIX --> G[read from nlri_data.prefix]
    D -- RESERVED --> H[keep defaults\nBGP_LS_PROTO_RESERVED / 0]
    E & F & G & H --> I{switch nlri_type\nDetermine nlri_type_str}
    I --> J{switch protocol_id\nDetermine protocol_str}
    J --> K[vty_out: NLRI Type, Protocol, Identifier]
    K --> L{switch nlri_type\nDetermine local_node ptr}
    L --> M[Display Local Node Descriptor fields]
    M --> N[Display NLRI-type-specific fields]

Loading

_{Reviews (1): Last reviewed commit: "bgpd: Fix wrong union member access in b..." | Re-trigger Greptile}