babeld: fix NULL pointer dereference in babel_clean_routing_process

[LyZephyr]

When babeld receives an exit signal (SIGINT/SIGTERM) during shutdown or when babel_routing_process is NULL (never initialized or already freed), the program crashes with SIGSEGV due to a NULL pointer dereference in babel_clean_routing_process(). The function accesses babel_routing_process->t_read and babel_routing_process->t_update without checking if babel_routing_process is NULL first.

This can occur when:

1. The program receives an exit signal before babel_routing_process is fully initialized
2. babel_routing_process initialization fails but the cleanup function is still called
3. The cleanup function is called multiple times

#0  __pthread_kill_implementation (no_tid=0, signo=11, threadid=140251119745472)
    at ./nptl/pthread_kill.c:44
#1  __pthread_kill_internal (signo=11, threadid=140251119745472)
    at ./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=140251119745472, signo=signo@entry=11)
    at ./nptl/pthread_kill.c:89
#3  0x00007f8ec27a0476 in __GI_raise (sig=sig@entry=11)
    at ../sysdeps/posix/raise.c:26
#4  0x00007f8ec2b23836 in core_handler (signo=11, siginfo=0x7ffd37d9db70,
    context=<optimized out>) at lib/sigevent.c:268
#5  <signal handler called>
#6  0x00007f8ec2b3ebdd in event_cancel (thread=0x8) at lib/event.c:1456
#7  0x000063ee3c9e7a3e in babel_clean_routing_process ()
    at babeld/babeld.c:320
#8  0x000063ee3c9e4636 in babel_exit_properly () at babeld/babel_main.c:306
#9  babel_sigexit () at babeld/babel_main.c:92
#10 0x00007f8ec2b23cca in frr_sigevent_process () at lib/sigevent.c:117
#11 0x00007f8ec2b3f93d in event_fetch (m=m@entry=0x63ee60c108a0,
    fetch=fetch@entry=0x7ffd37d9e9b0) at lib/event.c:1742
#12 0x00007f8ec2ab85d3 in frr_run (loop=0x63ee60c108a0) at lib/libfrr.c:1249
#13 0x000063ee3c9de8bb in main (argc=12, argv=0x7ffd37d9ec08)
    at babeld/babel_main.c:205

[greptile-apps]

Greptile Overview

Greptile Summary

Added NULL pointer check in babel_clean_routing_process() to prevent SIGSEGV crashes when cleanup is called before initialization completes or after the process has already been freed. The fix also sets babel_routing_process = NULL after freeing to prevent use-after-free issues on subsequent calls.

Key Changes:

• Added early return if babel_routing_process is NULL before accessing its fields (t_read, t_update, distribute_ctx)
• Set babel_routing_process = NULL after XFREE() to prevent dangling pointer issues

Impact:
Resolves crash scenarios when:

1. Exit signal received before babel_routing_process initialization completes
2. Socket creation fails during babel_create_routing_process() but cleanup is still invoked
3. Cleanup function called multiple times

The fix is minimal, defensive, and follows common NULL-checking patterns used throughout the codebase.

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The fix is a straightforward defensive programming improvement that adds a NULL check before dereferencing a pointer and sets the pointer to NULL after freeing. This is a common pattern for preventing NULL pointer dereferences and use-after-free bugs. The change is minimal, well-tested by the stack trace evidence, and directly addresses the reported crash scenario without introducing any side effects.
• No files require special attention

Important Files Changed

Filename	Overview
babeld/babeld.c	Added NULL check before accessing babel_routing_process and set pointer to NULL after freeing to prevent use-after-free

Sequence Diagram

sequenceDiagram
    participant Signal as Signal Handler
    participant Exit as babel_exit_properly()
    participant Clean as babel_clean_routing_process()
    participant BRP as babel_routing_process
    participant Event as event_cancel()

    Note over Signal,Event: Scenario: SIGINT/SIGTERM received

    Signal->>Exit: SIGINT or SIGTERM signal
    Exit->>Clean: Call cleanup function
    
    alt babel_routing_process is NULL
        Clean->>BRP: Check if babel_routing_process is NULL
        Note over Clean,BRP: Early return - prevents crash
        Clean-->>Exit: Return early
    else babel_routing_process is valid
        Clean->>BRP: Check if babel_routing_process is NULL
        Clean->>Event: event_cancel(&babel_routing_process->t_read)
        Clean->>Event: event_cancel(&babel_routing_process->t_update)
        Clean->>BRP: distribute_list_delete()
        Clean->>BRP: XFREE(babel_routing_process)
        Clean->>BRP: Set babel_routing_process = NULL
        Note over Clean,BRP: Prevents use-after-free on subsequent calls
        Clean-->>Exit: Return successfully
    end

Loading

[ton31337]

@Mergifyio backport dev/10.6 stable/10.5 stable/10.4 stable/10.3 stable/10.2

[mergify]

backport dev/10.6 stable/10.5 stable/10.4 stable/10.3 stable/10.2

✅ Backports have been created

Details

• #20849 babeld: fix NULL pointer dereference in babel_clean_routing_process (backport #20727) has been created for branch dev/10.6
• #20850 babeld: fix NULL pointer dereference in babel_clean_routing_process (backport #20727) has been created for branch stable/10.5
• #20851 babeld: fix NULL pointer dereference in babel_clean_routing_process (backport #20727) has been created for branch stable/10.4 but encountered conflicts

Cherry-pick of cb76f18 has failed:

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

You are currently cherry-picking commit cb76f1841.
  (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:   babeld/babeld.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

• #20852 babeld: fix NULL pointer dereference in babel_clean_routing_process (backport #20727) has been created for branch stable/10.3 but encountered conflicts

Cherry-pick of cb76f18 has failed:

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

You are currently cherry-picking commit cb76f1841.
  (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:   babeld/babeld.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

• #20853 babeld: fix NULL pointer dereference in babel_clean_routing_process (backport #20727) has been created for branch stable/10.2 but encountered conflicts

Cherry-pick of cb76f18 has failed:

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

You are currently cherry-picking commit cb76f1841.
  (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:   babeld/babeld.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

[donaldsharp]

you have a great analysis in the PR's comments, so why didn't you put the same thing in the commit message? Why would we not want to have that data preserrved there?

[LyZephyr]

you have a great analysis in the PR's comments, so why didn't you put the same thing in the commit message? Why would we not want to have that data preserrved there?

I’ve updated the commit message as suggested, thanks for the feedback.
For the CI: the Ubuntu 24.04 arm64 build failed while downloading some files, which looks like a network issue. I don’t seem to have permission to re-run the job from my side.

[riw777]

looks good