lib: Solve race condition in Link State

[odd22]

When a consumer request a TED synchronisation by calling ls_request_sync(), producer send TED elements with SYNC message which are in turn parse by ls_msg2ted() function or individual ls_msg2vertex(), ls_msg2edge() or ls_msg2subnet() function. In all cases, ls_xxx_add() functions are called. However, in some race condition, TED is not empty and already contains the same element that is under synchronisation. TED is an RB_TREE_UNIQ structure. Thus, TED is not allowed duplicate entry. The problem occurs when ls_xxx_add() functions try to add a sync element while it already exist. This element is not inserted in the TED and allocated memory lost at the end of caller leaving memory leaks when daemon stop.

This patch replace call to ls_xxx_add() functions by ls_xxx_update() functions when parsing SYNC message.

However, this patch is not solved the root cause which seems due to concurrent SYNC messages. Another PR will be proposed latter to solve the root cause.

[greptile-apps]

Greptile Summary

This PR addresses a memory leak in the Link State TED (Traffic Engineering Database) synchronization path. When a consumer calls ls_request_sync(), the producer sends TED elements tagged as LS_MSG_EVENT_SYNC; if those elements already exist in the RB-tree (due to a race with concurrent SYNC messages), the old ls_xxx_add() calls would allocate a new struct, fail the UNIQ RB-tree insertion silently, and return the dangling uninserted pointer — leaking both the wrapper struct and the payload data.

• Fix: Replaces ls_vertex_add, ls_edge_add, and ls_subnet_add with their _update counterparts in the three LS_MSG_EVENT_SYNC switch branches. The _update functions find any pre-existing entry, free or adopt the incoming payload correctly, and fall back to _add only when no duplicate exists.
• Scope: Only the SYNC event path is patched; the root cause (concurrent SYNC messages) is acknowledged as unresolved and deferred to a follow-up PR.

Confidence Score: 4/5

The change is a small, focused fix confined to three switch branches; the _update functions already existed and were used for UPDATE events, so their behaviour is well-proven. The ADD path still has the same underlying vulnerability but that is explicitly deferred to a follow-up PR.

The three changed lines correctly swap in functions that already handle the duplicate-entry case, memory is freed or adopted properly in both the matching and non-matching branches, and the SYNC status override is intentional and harmless. The only gap is the identical vulnerability still present in the ADD branches, which the author calls out as a follow-up item.

lib/link_state.c — specifically the LS_MSG_EVENT_ADD branches in ls_msg2vertex, ls_msg2edge, and ls_msg2subnet.

Important Files Changed

Filename	Overview
lib/link_state.c	Three one-line changes swap ls_xxx_add → ls_xxx_update in SYNC event handlers, correctly fixing the duplicate-entry memory leak while preserving ADD/UPDATE/DELETE semantics unchanged.

Sequence Diagram

sequenceDiagram
    participant C as Consumer
    participant P as Producer
    participant TED as TED (RB-tree)

    C->>P: ls_request_sync()
    P->>C: LS_MSG_EVENT_SYNC (vertex/edge/subnet)
    C->>C: ls_msg2vertex / ls_msg2edge / ls_msg2subnet

    alt Element does NOT exist (normal path)
        C->>TED: ls_xxx_update() calls ls_xxx_add()
        TED-->>C: "new element inserted, status=SYNC"
    else Element ALREADY EXISTS (race condition, fixed by this PR)
        C->>TED: ls_xxx_update() finds existing entry
        TED-->>C: existing element returned, payload freed or adopted
        C->>TED: "set status=SYNC on existing element"
    end

Loading

Comments Outside Diff (1)

1. lib/link_state.c, line 1932-1934 (link)

   LS_MSG_EVENT_ADD shares the same duplicate-entry memory leak

   The LS_MSG_EVENT_ADD branches in all three functions (ls_msg2vertex, ls_msg2edge, ls_msg2subnet) still call ls_xxx_add() directly. If a duplicate ADD message arrives (e.g. during a replay or the concurrent-message race), ls_vertex_add silently allocates a new struct, fails the UNIQ RB-tree insertion, and returns the dangling uninserted pointer — leaking the wrapper and its payload in exactly the same way the SYNC path did before this fix. The PR description acknowledges the root cause is deferred, but worth tracking since the same pattern exists here.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: lib/link_state.c
   Line: 1932-1934
   
   Comment:
   **`LS_MSG_EVENT_ADD` shares the same duplicate-entry memory leak**
   
   The `LS_MSG_EVENT_ADD` branches in all three functions (`ls_msg2vertex`, `ls_msg2edge`, `ls_msg2subnet`) still call `ls_xxx_add()` directly. If a duplicate ADD message arrives (e.g. during a replay or the concurrent-message race), `ls_vertex_add` silently allocates a new struct, fails the UNIQ RB-tree insertion, and returns the dangling uninserted pointer — leaking the wrapper and its payload in exactly the same way the SYNC path did before this fix. The PR description acknowledges the root cause is deferred, but worth tracking since the same pattern exists here.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
lib/link_state.c:1932-1934
**`LS_MSG_EVENT_ADD` shares the same duplicate-entry memory leak**

The `LS_MSG_EVENT_ADD` branches in all three functions (`ls_msg2vertex`, `ls_msg2edge`, `ls_msg2subnet`) still call `ls_xxx_add()` directly. If a duplicate ADD message arrives (e.g. during a replay or the concurrent-message race), `ls_vertex_add` silently allocates a new struct, fails the UNIQ RB-tree insertion, and returns the dangling uninserted pointer — leaking the wrapper and its payload in exactly the same way the SYNC path did before this fix. The PR description acknowledges the root cause is deferred, but worth tracking since the same pattern exists here.

_{Reviews (1): Last reviewed commit: "lib: Solve race condition in Link State" | Re-trigger Greptile}