[PATCH 0/2] Fix domain assignment TOCTOU races

[Anup Patel]

On Wed, Aug 19, 2026 at 1:22 PM Huamao Wu <huamao.wu at linux.spacemit.com> wrote:
>
> The per-hart domain assignment is represented by two fields that must
> stay consistent at all times:
>
>   1. a per-hart pointer stored in sbi_scratch (hartindex_to_domain)
>   2. a per-domain bitmap (assigned_harts)
>
> A concurrent reader is correct only if it observes both fields in
> agreement.  Today these fields are updated independently, creating
> TOCTOU windows where a hart transiently belongs to no domain or to
> two domains.
>
> These races were identified through code review.  On our internal
> platform we observed symptoms consistent with the cross-call TOCTOU
> described in Race 2 (a HART_START ecall seeing an assignment that a
> preceding HART_GET_STATUS did not), though the root cause of those
> specific symptoms turned out to be an unrelated cache-coherency issue.
> The code paths nonetheless present TOCTOU windows worth addressing
> regardless of the observed root cause.
>
> Race 1: switch_to_next_domain_context() non-atomic reassignment
> ---------------------------------------------------------------
>
> The context-switch path updates the two fields in three separate
> locked steps.  A concurrent HART_START that checks ownership between
> steps sees an inconsistent state:
>
>   1. Hart A: clear hart N from old_dom->assigned_harts
>   2. Hart A: update hartindex_to_domain[hart N] = new_dom
>              -- hart N is in no domain's bitmap --
>   3. Hart B: check assigned_harts for hart N -> not found
>              -- hart appears unowned, HART_START rejected --
>   4. Hart A: set hart N in new_dom->assigned_harts
>
> Between steps 1 and 4 the hart is in no domain.  A concurrent
> HART_START that validates ownership at step 3 will incorrectly
> reject the hart.
>
> Race 2: sbi_hsm_hart_start() check-vs-transition gap
> -----------------------------------------------------
>
> The HART_START handler validates domain ownership, acquires the start
> ticket, and atomically transitions the hart to START_PENDING in three
> unprotected steps.  A concurrent domain assignment can reassign the
> hart between the check and the commit:
>
>   1. Hart A: check dom->assigned_harts for hart N -> OK
>   2. Hart B: assign_hart(hart N, other_dom)
>              -- hart N now belongs to other_dom --
>   3. Hart A: acquire start ticket
>   4. Hart A: cmpxchg state STOPPED -> START_PENDING
>              -- hart N starts in a domain that no longer owns it --
>
> The ownership check and the state transition are not serialized
> against concurrent assignment changes, so HART_START can commit for
> a domain that lost ownership between check and commit.
>
> Patch 1 introduces the locking infrastructure and converts the
> assignment path in sbi_domain_register().
>
> Patch 2 converts the two remaining callers (sbi_hsm_hart_start and
> switch_to_next_domain_context) to the atomic API.
>
> The patches are also available on GitHub:
>
>   https://github.com/kasperis7/opensbi branch domain-assignment-race-fix
>
> Huamao Wu (2):
>   lib: sbi_domain: introduce atomic hart assignment helper
>   lib: sbi: serialize HSM hart_start ownership check with assignment
>     lock
>
>  include/sbi/sbi_domain.h     | 19 +++++++++
>  lib/sbi/sbi_domain.c         | 81 ++++++++++++++++++++++++++++++++++------
>  lib/sbi/sbi_domain_context.c | 13 +------
>  lib/sbi/sbi_hsm.c            | 15 +++++--
>  4 files changed, 108 insertions(+), 20 deletions(-)
>
> --
> 2.43.0
>

Overall motivation and problem statement of this series is good but
patch organization is not correct.

The first patch should be to improve sbi_update_hartindex_to_domain()
as show below whereas second patch should only add stuff required
to serialize HSM hart_start.

diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
index b3f5892d..288b7005 100644
--- a/lib/sbi/sbi_domain.c
+++ b/lib/sbi/sbi_domain.c
@@ -37,27 +37,49 @@ struct sbi_domain root = {
 };

 static unsigned long domain_hart_ptr_offset;
+static DEFINE_SPIN_LOCK(domain_hart_assign_lock);

 struct sbi_domain *sbi_hartindex_to_domain(u32 hartindex)
+ MUST_NOT_HOLD(&domain_hart_assign_lock)
 {
  struct sbi_scratch *scratch;
+ struct sbi_domain *dom;

  scratch = sbi_hartindex_to_scratch(hartindex);
  if (!scratch || !domain_hart_ptr_offset)
  return NULL;

- return sbi_scratch_read_type(scratch, void *, domain_hart_ptr_offset);
+ spin_lock(&domain_hart_assign_lock);
+ dom = sbi_scratch_read_type(scratch, void *, domain_hart_ptr_offset);
+ spin_unlock(&domain_hart_assign_lock);
+ return dom;
 }

 void sbi_update_hartindex_to_domain(u32 hartindex, struct sbi_domain *dom)
+ MUST_NOT_HOLD(&domain_hart_assign_lock)
+ MUST_NOT_HOLD(&dom->assigned_harts_lock)
 {
+ struct sbi_domain *current_dom;
  struct sbi_scratch *scratch;

  scratch = sbi_hartindex_to_scratch(hartindex);
- if (!scratch)
+ if (!scratch || !domain_hart_ptr_offset)
  return;

+ spin_lock(&domain_hart_assign_lock);
+ current_dom = sbi_scratch_read_type(scratch, void *, domain_hart_ptr_offset);
+ if (current_dom) {
+ spin_lock(&current_dom->assigned_harts_lock);
+ sbi_hartmask_clear_hartindex(hartindex, &current_dom->assigned_harts);
+ spin_unlock(&current_dom->assigned_harts_lock);
+ }
  sbi_scratch_write_type(scratch, void *, domain_hart_ptr_offset, dom);
+ if (dom) {
+ spin_lock(&dom->assigned_harts_lock);
+ sbi_hartmask_set_hartindex(hartindex, &dom->assigned_harts);
+ spin_unlock(&dom->assigned_harts_lock);
+ }
+ spin_unlock(&domain_hart_assign_lock);
 }

 bool sbi_domain_is_assigned_hart(const struct sbi_domain *dom, u32 hartindex)
@@ -687,7 +709,6 @@ int sbi_domain_register(struct sbi_domain *dom)
  }

  sbi_update_hartindex_to_domain(i, dom);
- sbi_hartmask_set_hartindex(i, &dom->assigned_harts);

  /*
  * If cold boot HART is assigned to this domain then
diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
index 987ac7fa..ecc20fe8 100644
--- a/lib/sbi/sbi_domain_context.c
+++ b/lib/sbi/sbi_domain_context.c
@@ -120,17 +120,10 @@ static int switch_to_next_domain_context(struct
hart_context *ctx,

  current_dom = ctx->dom;
  target_dom = dom_ctx->dom;
- /* Assign current hart to target domain */
- spin_lock(&current_dom->assigned_harts_lock);
- sbi_hartmask_clear_hartindex(hartindex, &current_dom->assigned_harts);
- spin_unlock(&current_dom->assigned_harts_lock);

+ /* Assign current hart to target domain */
  sbi_update_hartindex_to_domain(hartindex, target_dom);

- spin_lock(&target_dom->assigned_harts_lock);
- sbi_hartmask_set_hartindex(hartindex, &target_dom->assigned_harts);
- spin_unlock(&target_dom->assigned_harts_lock);
-
  /* Save current CSR context and restore target domain's CSR context */
  ctx->sstatus = csr_swap(CSR_SSTATUS, dom_ctx->sstatus);
  ctx->sie = csr_swap(CSR_SIE, dom_ctx->sie);

Regards,
Anup