Skip to content

Fix je_malloc_underlying_allocation to correctly validate sub-capabilities #2446

@MauriceDHanisch

Description

@MauriceDHanisch

Description:

While investigating je_malloc_underlying_allocation we found that its current implementation relies too heavily on the input pointer (as noted in existing comments). It only checks that the offset is non-zero and then proceeds, which means a sub-capability derived from a valid allocation with adapted bounds can be passed to free(). This undermines heap spatial safety (as described in Brett Gutstein’s dissertation). It can also be used to break temporal safety, since freeing a sub-capability causes the quarantine logic to treat the wrong region as freed, unpainting shadow bits for addresses that still belong to quarantined allocations.

There seems to be two possible design choices:

  1. Accept subcapabilities:
    Treat any capability derived from an allocation as valid, look up the owning extent, and return a capability to the underlying allocation. This ensures free/underlying_allocation works even if passed a subcapability.

    • We implemented this in branch get-underlying-allocation.
    • This version validates the capability (tag, bounds, seal bit), then uses the rtree and size class arithmetic to compute the correct region base inside the extent.
  2. Reject subcapabilities:
    Enforce that only the original allocation capability may be passed. If a subcapability is provided, the allocator traps with an error.

    • We implemented this in branch check-underlying-allocation.
    • This version validates the capability and explicitly aborts if it does not point to the base of an allocation region.

Testing:

Both branches were tested with the CheriBSD lib/libc/stdlib Kyua suite: kyua test -k /usr/tests/Kyuafile lib/libc/stdlib. Results were unchanged compared to the current tree: all tests passed except posix_memalign_test:aligned_alloc_basic and posix_memalign_basic.

Discussion point:

It’s unclear what the intended contract is: should the allocator be permissive (resolve subcaps) or strict (trap on non-base capabilities)? We would appreciate guidance on which direction is preferred before turning this into a PR.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions