Bluetooth: Controller: Use-after-release of lll_event in mayfly_enqueue path in lll_scan.c/lll_scan_aux.c

[cvinayak]

Describe the bug

Bug Description

A use-after-release (use-after-free) scenario exists in Zephyr's Bluetooth Controller, specifically in the scan and scan_aux LLL pipeline. The issue concerns the scheduling of ull_sched_mfy_after_cen_offset_get() using mayfly_enqueue from within lll_scan.c and lll_scan_aux.c, where a pointer to an event structure (likely a stack-allocated or MFIFO-allocated lll_event) is passed to the mayfly mechanism for deferred execution in the ULL_LOW context.

Control Path Details

1. End of scan event:
   • lll_done(void *param) is called at the end of an LLL event (e.g., scan event). It marks the event as complete, releases resources, and may schedule a resume via mayfly if the event was preempted.
2. Pipeline Management:
   • ull_prepare_dequeue_get() and then MFIFO_DEQUEUE(prep) are used to dequeue and release the event buffer for reuse. This happens immediately after mayfly scheduling.
3. Preempted Event Handling and Resume:
   • If preempted, mayfly_enqueue(..., lll_resume, lll_event*) is called to schedule a deferred resume. lll_resume(void *param) is called from mayfly, and invokes lll_prepare_resolve() with the event context.
4. Scan Prepare Callback:
   • Functions like lll_scan_prepare(void *param) handle scan event programming. If a connection is established, they prepare to schedule ULL actions.
5. Scheduling ULL_LOW Callback:
   • In lll_scan.c or lll_scan_aux.c:

     if (lll->conn) {
         static memq_link_t link;
         static struct mayfly mfy_after_cen_offset_get = { 0U, 0U, &link, NULL, ull_sched_mfy_after_cen_offset_get };
         mfy_after_cen_offset_get.param = p;
         retval = mayfly_enqueue(TICKER_USER_ID_LLL, TICKER_USER_ID_ULL_LOW, 1U, &mfy_after_cen_offset_get);
         LL_ASSERT(!retval);
     }

     This schedules ull_sched_mfy_after_cen_offset_get() to run in ULL_LOW context, passing a pointer to the current event/context (p).
6. Immediate Event Release:
   • After scheduling, MFIFO_DEQUEUE(prep) is called, releasing the buffer for reuse.
7. Deferred Callback Execution:
   • ull_sched_mfy_after_cen_offset_get(void *param) runs in ULL_LOW context and dereferences the pointer passed, which may now point to a recycled buffer if another event was scheduled in the interim.

Path Summary with Function Call Chain

1. lll_done() → (pipeline management) → if preempted: mayfly_enqueue(..., lll_resume, lll_event*)
2. lll_resume(lll_event*) → lll_prepare_resolve() → scan prepare callback (e.g., lll_scan_prepare) → if lll->conn, then: mayfly_enqueue(TICKER_USER_ID_LLL, TICKER_USER_ID_ULL_LOW, ..., ull_sched_mfy_after_cen_offset_get, lll_event*) → (immediately after): event buffer is released (MFIFO_DEQUEUE)
3. Later in ULL_LOW context: ull_sched_mfy_after_cen_offset_get(lll_event*)

Critical Window:
Between mayfly_enqueue of ull_sched_mfy_after_cen_offset_get() and the execution of this callback, the pointer may be released and reused for a new event, leading to a use-after-release scenario.

Impact

• This is a memory safety bug that could cause subtle and hard-to-reproduce failures, crashes, or security vulnerabilities in Bluetooth Controller operation.

Steps to Reproduce

• Run continuous scanning with rapid preemption and rescheduling, or any case where scan events rapidly follow each other with close timing.
• Instrument the code to observe reuse of MFIFO slots and deferred execution of mayfly callbacks.

Additional Context

• The lifetime of the lll_event structure (or similar context) must be guaranteed until the mayfly callback has executed. Currently, the dequeue/release happens too soon.
• Any code refactoring or fix should defer MFIFO_DEQUEUE or otherwise guarantee memory validity until after the mayfly callback.

---

Relevant labels: bug, area: Bluetooth Controller, area: Memory Management, area: Bluetooth

Regression

• This is a regression.

Steps to reproduce

No response

Relevant log output

Impact

Functional Limitation – Some features not working as expected, but system usable.

Environment

No response

Additional Context

No response

[cvinayak]

Example GPT-5 Prompt (not the exact one tho that is copied but close to it):

The use-after-release is only probably when scheduling using mayfly_enqueue from high priority execution context to lower execution context. And the use-after-release for lll_event structure being passed. Hence, revise your analysis and give detailed literal code locations and full path excerpts

and

Yes, give me full function call traces and verify other locations

Answer:

Below are the full function call traces and verified locations where a pointer originating from the LLL prepare pipeline (via a resumed event) eventually results in a mayfly call from a higher-priority LLL context to a lower-priority ULL_LOW context. These are the scenarios that can cause a use-after-release if the prepare FIFO element (commonly carrying the event context) is dequeued and reused before the deferred ULL_LOW callback runs.

Important note on completeness: the code-search results used to assemble this are limited. For broader coverage, please also review in the GitHub UI with repository-wide search for mayfly calls to ULL_LOW.

High-to-Low (LLL → ULL_LOW) mayfly scheduling that carries event context

1. Nordic: lll_scan.c — LLL schedules ULL_LOW callback after prepare

• Control flow:

  • lll_done (LLL) → mayfly to lll_resume (LLL) → lll_resume → lll_prepare_resolve → scan prepare callback → mayfly to ULL_LOW: ull_sched_mfy_after_cen_offset_get

• Resume entrypoint (LLL), called with the lll_event pointer previously scheduled, immediately after the prepare FIFO element was enqueued:

void lll_resume(void *param)
{
    struct lll_event *next;
    int err;

    next = param;
    err = lll_prepare_resolve(next->is_abort_cb, next->abort_cb, next->prepare_cb,
                              &next->prepare_param, next->is_resume, 1U);
    LL_ASSERT(!err || err == -EINPROGRESS);
}

• The ULL loop that schedules the resume and immediately dequeues the prepare FIFO element (releasing the buffer for reuse):

// ... inside the prepare loop
mfy.param = next; // 'next' is struct lll_event*
ret = mayfly_enqueue(caller_id, TICKER_USER_ID_LLL, 0, &mfy);
LL_ASSERT(!ret);

MFIFO_DEQUEUE(prep); // prepare FIFO element released for reuse, immediately after scheduling

// loop continues ...

• In the scan prepare path, when a connection is established, the code schedules a ULL_LOW callback from LLL:

if (lll->conn) {
    static memq_link_t link;
    static struct mayfly mfy_after_cen_offset_get = {
        0U, 0U, &link, NULL, ull_sched_mfy_after_cen_offset_get};
    uint32_t retval;

    mfy_after_cen_offset_get.param = p;

    retval = mayfly_enqueue(TICKER_USER_ID_LLL, TICKER_USER_ID_ULL_LOW, 1U,
                            &mfy_after_cen_offset_get);
    LL_ASSERT(!retval);
}

• Notes:
  • The pointer in mfy_after_cen_offset_get.param is the active context for the just-prepared/resumed scan. It originates from the prepare pipeline path that used the lll_event container (‘next’ above) before MFIFO_DEQUEUE released it.
  • Because the resume path’s lll_event is dequeued immediately after mayfly scheduling (see ull.c above), there is a window where the context backing ‘p’ can be recycled before the ULL_LOW mayfly executes.

2. Nordic: lll_scan_aux.c — LLL schedules ULL_LOW callback after auxiliary scan prepare

• Control flow:

  • lll_done (LLL) → mayfly to lll_resume (LLL) → lll_resume → lll_prepare_resolve → lll_scan_aux prepare callback → mayfly to ULL_LOW: ull_sched_mfy_after_cen_offset_get

• LLL-side ULL_LOW scheduling:

ret = mayfly_enqueue(TICKER_USER_ID_LLL, TICKER_USER_ID_ULL_LOW, 1U,
                     &mfy_after_cen_offset_get);
LL_ASSERT(!ret);

• Notes:
  • Same lifetime window exists: a prepare FIFO element (container for the resume/prepare context) is dequeued immediately after mayfly scheduling to LLL; then, the LLL code schedules ULL_LOW execution with a pointer to that context, risking use-after-release if a new prepare reuses the buffer before the ULL_LOW mayfly runs.

Additional call-chain anchors for context and lifetime

• lll_done (resets current LLL event; the path to resume comes via ULL’s prepare pipeline):

int lll_done(void *param)
{
    struct lll_event *next;
    struct ull_hdr *ull;
    void *evdone;

    // ...
    if (!param) {
        LL_ASSERT(event.curr.abort_cb);
        event.curr.abort_cb = NULL;

        param = event.curr.param;
        event.curr.param = NULL;

        // param → ull resolution (Resume events set curr.param = NULL)
        // ...
    } else {
        ull = HDR_LLL2ULL(param);
    }

    // dequeue from prepare pipeline or done-counter bookkeeping
    // ...
    evdone = ull_event_done(ull);
    LL_ASSERT(evdone);
    return 0;
}

• The ULL prepare pipeline loop that both schedules the LLL resume and dequeues (releases) the prepare FIFO entry:

// ...
next = ull_prepare_dequeue_get();
while (next) {
    // read out fields that will be consumed (ticks, param, flags)
    // ...
    if (!is_aborted) {
        static memq_link_t link;
        static struct mayfly mfy = {0, 0, &link, NULL, lll_resume};

        mfy.param = next; // pass the struct lll_event* to LLL resume
        ret = mayfly_enqueue(caller_id, TICKER_USER_ID_LLL, 0, &mfy);
        LL_ASSERT(!ret);
    }

    MFIFO_DEQUEUE(prep); // release ‘next’ for reuse immediately after scheduling

    // continue iterating
    next = ull_prepare_dequeue_get();
    // ...
}

Why these are the high-to-low cases that matter

• The risk you highlighted is specifically when scheduling from a higher-priority execution context (LLL) down to ULL_LOW. The two verified instances are in the Nordic scan paths:
  • nordic/lll/lll_scan.c
  • nordic/lll/lll_scan_aux.c
• Other mayfly uses we examined either:
  • Stay within LLL or go from LLL to ULL_HIGH (not part of your narrowed risk), or
  • Don’t pass the prepare-pipeline-backed event/context as the mayfly parameter.

Direct links to expand the search (UI)

• Search for LLL → ULL_LOW mayfly scheduling:
  • Search: TICKER_USER_ID_ULL_LOW in ll_sw
  • Search: mayfly_enqueue in lll_scan.c
  • Search: mayfly_enqueue in lll_scan_aux.c

Summary of call traces (risk window highlighted)

• lll_done (LLL) → ULL prepare pipeline → mayfly to lll_resume (LLL), then MFIFO_DEQUEUE releases lll_event buffer
• lll_resume (LLL) → lll_prepare_resolve → scan/aux prepare callback (LLL)
• scan/aux prepare (LLL) → mayfly to ULL_LOW (ull_sched_mfy_after_cen_offset_get) with pointer to the just-used context
• Window: between this LLL→ULL_LOW mayfly enqueue and actual ULL_LOW execution, a new prepare can reuse the buffer that held the context pointer (potential use-after-release)

If you want me to enumerate any further LLL→ULL_LOW mayfly instances beyond these two Nordic scan paths, point me to a specific subdirectory or role and I’ll trace those as well.

[cvinayak]

Another prompt:

Please analyse whether this PR fixes the use-after-release issue? @zephyrproject-rtos/zephyr/pull/98024

Answer:

Short answer: Yes, this PR addresses the specific use-after-release you identified in lll_scan/lll_scan_aux when scheduling ull_sched_mfy_after_cen_offset_get() from LLL to ULL_LOW.

What the bug was

• The LLL resume/prepare path uses a prepare FIFO entry (struct lll_event) that is mayfly-scheduled to LLL and then immediately dequeued (released) for reuse.
• The scan code then mayfly-scheduled ull_sched_mfy_after_cen_offset_get() from LLL to ULL_LOW passing a pointer derived from that same transient prepare context (p), creating a window where the pointer could be recycled before the ULL_LOW callback executed.

What this PR changes

• It stops passing the transient prepare context (p) to the ULL_LOW mayfly and instead copies the needed values into stable storage within the scan instance (struct lll_scan).
• New field added (guarded appropriately so it only exists when used):
  • subsys/bluetooth/controller/ll_sw/lll_scan.h
    • Adds struct lll_prepare_param prepare_param in struct lll_scan under CONFIG_BT_CTLR_SCHED_ADVANCED.
• Updated callers to populate and pass this persistent storage:
  • subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c
    • Instead of mfy_after_cen_offset_get.param = p, it fills lll->prepare_param with ticks_at_expire, remainder, and param = lll, then sets mfy_after_cen_offset_get.param = &lll->prepare_param.
  • subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c
    • Mirrors the same change for the auxiliary scan path.

Why this fixes the UAF

• The pointer handed to ULL_LOW is no longer the prepare FIFO’s transient object nor anything derived from the immediately dequeued lll_event. It now points to memory owned by the lll_scan instance whose lifetime extends safely beyond the deferred mayfly execution.
• The prepare FIFO’s immediate MFIFO_DEQUEUE remains as-is (in ull.c), but that no longer affects the parameter passed to the ULL_LOW callback, eliminating the use-after-release window for this path.

Scope and residual risks

• This PR directly fixes the two LLL→ULL_LOW call sites that were problematic:
  • Nordic lll_scan.c
  • Nordic lll_scan_aux.c
• The changes are guarded by CONFIG_BT_CENTRAL && CONFIG_BT_CTLR_SCHED_ADVANCED, matching the code paths that schedule ull_sched_mfy_after_cen_offset_get().
• If there are other LLL→ULL_LOW mayfly call sites that still pass pointers derived from the prepare FIFO (we previously only found these two), they would need the same treatment. Based on the diffs for this PR, those two are the ones addressed.

Minor suggestion (non-blocking for correctness)

• The new lll_scan.prepare_param field could benefit from a brief comment documenting that it stores a copy of the prepare parameters specifically to survive deferred ULL_LOW execution and avoid use-after-release of the prepare FIFO context.

Verdict

• The fix is correct for the identified use-after-release in lll_scan and lll_scan_aux: it replaces a short-lived parameter with a long-lived copy anchored in the lll_scan instance, removing the lifetime hazard between LLL scheduling and ULL_LOW execution.

[cvinayak]

Prompt:

Can you analyse potential undesirable side effects of this PR? Will the change is likely to cause any of the unit and regression tests to fail. There are BabbleSIM tests run in the PR CI, you can use this information? i.e. testcases passing before and after the PR?

Answer:

Here’s an impact analysis of PR “Bluetooth: Controller: Fix use-after-release in lll_scan/lll_scan_aux” and whether it’s likely to break tests (including BabbleSim):

PR summary and what it changes

• Problem fixed: Avoid passing a transient prepare FIFO object (p / lll_event-backed) from LLL to ULL_LOW via mayfly when computing connection window offset (ull_sched_mfy_after_cen_offset_get). That transient object could be dequeued and reused before the ULL_LOW mayfly runs.
• Fix approach: Add a persistent field to the scan LLL instance (struct lll_scan.prepare_param) and copy only the needed values (ticks_at_expire, remainder, param=lll) into it before mayfly_enqueue. The mayfly now receives &lll->prepare_param (stable memory).
• Files modified:
  • subsys/bluetooth/controller/ll_sw/lll_scan.h: add struct lll_prepare_param prepare_param under CONFIG_BT_CTLR_SCHED_ADVANCED.
  • subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c: copy needed fields into lll->prepare_param; pass its address to mayfly_enqueue.
  • subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c: same as above for the auxiliary scan path.

Why the fix is correct and low-risk

• ull_sched_mfy_after_cen_offset_get(void param) expects param to be struct lll_prepare_param. It uses:
  • p->ticks_at_expire
  • p->remainder
  • p->param (cast to struct lll_scan*)
    It does not use lazy/force/prio, so copying only these three fields is sufficient and semantically equivalent to the prior usage.
• The lifetime hazard is eliminated because lll->prepare_param lives as long as the lll_scan instance, outliving the deferred mayfly execution.

Potential undesirable side effects

• Slight memory increase: Each scan instance (struct lll_scan) grows by sizeof(struct lll_prepare_param) when CONFIG_BT_CTLR_SCHED_ADVANCED is enabled. On 32-bit, this is typically in the ~16–24 byte range. There are at most two scan instances (1M and Coded), so impact is small.
• Overwrite risk if multiple deferred ULL_LOW callbacks are scheduled for the same lll_scan before the first one executes:
  • The fix uses a single slot (lll->prepare_param) per scan instance. If two LLL→ULL_LOW mayflies for ull_sched_mfy_after_cen_offset_get are enqueued back-to-back for the same lll before ULL_LOW runs, the later copy would overwrite the earlier.
  • In practice, this path triggers upon connection establishment (lll->conn != NULL). Once a connection is formed, scanning for that set stops (or is quiesced), so concurrent scheduling of multiple “after_cen_offset_get” callbacks for the same lll is not expected. Also, 1M vs Coded have separate lll_scan instances, so they don’t collide.
  • Conclusion: overwrites are theoretically possible but practically unlikely in the intended flow; significantly safer than the prior use-after-release.
• Concurrency/memory-ordering concerns:
  • LLL writes lll->prepare_param then calls mayfly_enqueue. ULL_LOW reads it later. There’s no explicit memory barrier, but this mirrors existing patterns in controller code. Typical single-core BLE controller setups are fine. If SMP LLL/ULL were to be enabled, that subsystem already relies on mayfly’s ordered handoff.
• Structure layout changes:
  • struct lll_scan grows under CONFIG_BT_CTLR_SCHED_ADVANCED. This struct is internal to controller; no external ABI concerns.

Test impact and BabbleSim (bsim) expectations

• Functional equivalence: The algorithm and values used by ull_sched_mfy_after_cen_offset_get are unchanged; only the storage location of the prepare parameters changes. Therefore, tests asserting correct connection window offset computation should continue to pass.
• Regression risk: Minimal for paths gated by CONFIG_BT_CENTRAL && CONFIG_BT_CTLR_SCHED_ADVANCED. Other configurations are unaffected.
• BabbleSim tests:
  • These PRs run BabbleSim in CI. The modified code paths are exercised by central-initiated connection scenarios (e.g., tests that scan and connect under advanced scheduler).
  • Based on the change (storage-only refactor for the mayfly param), there’s low likelihood of BabbleSim regressions. If tests were previously flaky due to the UAF, this should reduce flakiness.
  • I cannot fetch CI check results via this interface. Please review the PR Checks tab for pass/fail deltas, especially bsim Bluetooth central tests (scan + connect with advanced scheduler enabled).

Additional suggestions

• Add a brief comment in struct lll_scan near prepare_param explaining it stores the prepare parameters for deferred mayfly execution to avoid UAF (one review comment already suggests this).
• If future changes introduce multiple concurrent ULL_LOW callbacks using prepare_param for the same lll, consider either:
  • A small ring of two entries, or
  • Guarding to avoid enqueueing a second callback until the first is consumed.

Conclusion

• This PR is the correct fix for the identified use-after-release in lll_scan/lll_scan_aux. It is low-risk, minimally invasive, and should not break unit/regression/BabbleSim tests. If anything, it should improve stability by removing a race.

Full PR returned from GitHub API

data:
- url: "https://github.com/zephyrproject-rtos/zephyr/pull/98024"
  state: "open"
  draft: true
  title: "Bluetooth: Controller: Fix use-after-release in lll_scan/lll_scan_aux"
  number: 98024
  created_at: ""
  closed_at: ""
  merged_at: ""
  labels: []
  author: "cvinayak"
  comments: 0
  assignees_avatar_urls:
- "https://avatars.githubusercontent.com/u/6350656?v=4"