[PATCH 0/2] Fix domain assignment TOCTOU races

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


-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi