Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions kernel/data/vm/vmblock.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ typedef struct H2K_vmblock_struct {
/* Pointer to thread context storage */
H2K_thread_context *contexts;

/* Set under BKL when main is exiting; gates self-reap in resched. */
u32_t exiting;

union {
u32_t flags;
struct {
Expand Down
26 changes: 0 additions & 26 deletions kernel/event/intpool/test_h2/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,12 @@ int main()
pthread_attr_init(&pt_attrs);
pthread_attr_setschedparam(&pt_attrs,&lowprio_param);
pthread_create(&timeout_child,&pt_attrs,timeout,NULL);
#ifdef USE_DETACHED_THREADS
pthread_detach(timeout_child);
#endif
/* Configure interrupts as intpool interrupts */
for (i = INT_START; i < INT_LAST; i++) {
h2_intpool_config(i,1);
}
/* Create one intpool thread */
pthread_create(&intpool_child_0,NULL,worker,(void*)&seen_0);
#ifdef USE_DETACHED_THREADS
pthread_detach(intpool_child_0);
#endif
/* Trigger interrupt */
h2_hwconfig_hwintop(HWCONFIG_HWINTOP_RAISE,INT_START,1);
/* Check that intpool thread (down sem?) happened */
Expand Down Expand Up @@ -143,9 +137,6 @@ int main()

/* Create another intpool thread */
pthread_create(&intpool_child_1,NULL,worker,(void*)&seen_1);
#ifdef USE_DETACHED_THREADS
pthread_detach(intpool_child_1);
#endif
/* Check that both intpool threads can get interrupted */
/* Trigger more interrupts */
for (i = INT_START; i < INT_LAST; i++) {
Expand All @@ -172,23 +163,6 @@ int main()
for (i = 0; i < 1024; i++) { asm volatile (""); }
if (seen_0 || seen_1) FAIL("should not have seen anything after disabling.");

puts("Trying to stop all created threads");
stop_threads = 1;
h2_intpool_config(INT_START,1);
h2_hwconfig_hwintop(HWCONFIG_HWINTOP_RAISE,INT_START,1); // trigger interrupt to fast exit from worker thread
h2_sem_down(&int_received_sem);
h2_intpool_config(INT_START,1);
h2_hwconfig_hwintop(HWCONFIG_HWINTOP_RAISE,INT_START,1); // trigger interrupt to fast exit from worker thread
h2_sem_down(&int_received_sem);
puts("Awake last time");

void *ret;
pthread_join(timeout_child, &ret);
puts("Joining time-out thread");
pthread_join(intpool_child_0, &ret);
puts("Joining worker 0 thread");
pthread_join(intpool_child_1, &ret);
puts("Joining worker 1 thread");
puts("TEST PASSED");
return 0;
}
Expand Down
3 changes: 2 additions & 1 deletion kernel/event/trap/test/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ s32_t H2K_thread_id() { return 1; }
s32_t H2K_thread_create() { return 4; }
s32_t H2K_thread_stop() { return 5; }
s32_t H2K_cputime_get() { return 6; }
s32_t H2K_vm_stop() { return 7; }
s32_t H2K_register_fastint() { return 8; }
s32_t H2K_prio_set() { return 9; }
s32_t H2K_prio_get() { return 10; }
Expand Down Expand Up @@ -81,7 +82,7 @@ void user_mode();
u64_t guest_stack[128] __attribute__((aligned(128*8)));

s32_t testvals[] = {
0, 1, 2, 3, 4, 5, 6, 1, 8, 9,10,11,12,13,14,15,
0, 1, 2, 3, 4, 5, 6, 7, 8, 9,10,11,12,13,14,15,
16, 1,18,19,20,21,22,23,24,25,26,27,28,29,30,31
};

Expand Down
4 changes: 2 additions & 2 deletions kernel/event/trap/trap.ref.S
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ FUNC_START H2K_traptab .text.core.trap0 .p2align 8
jump H2K_cputime_get // 6
}
{
r0 = r7
jump H2K_thread_id // 7 // jumpr r31?
r1 = r7
jump H2K_vm_stop // 7
}
{
r2 = r7
Expand Down
4 changes: 2 additions & 2 deletions kernel/event/trap/trap.v3opt.S
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ FUNC_START H2K_traptab .text.core.trap0 .p2align 8
jump H2K_cputime_get // 6
}
{
r0 = sgp
jump H2K_thread_id // 7 // jumpr r31?
r1 = sgp
jump H2K_vm_stop // 7
}
{
r2 = sgp
Expand Down
4 changes: 2 additions & 2 deletions kernel/event/trap/trap.v4opt.S
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ FUNC_START H2K_traptab .text.core.trap0 .p2align 8
jump H2K_cputime_get // 6
}
{
r0 = sgp0
jump H2K_thread_id // 7 // jumpr r31?
r1 = sgp0
jump H2K_vm_stop // 7
}
{
r2 = sgp0
Expand Down
16 changes: 14 additions & 2 deletions kernel/sched/resched/resched.ref.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,23 @@
#include <resched.h>
#include <globals.h>
#include <log.h>
#include <timer.h>
#include <stop.h>

static inline void resched(u32_t unused, H2K_thread_context *me, u32_t hwtnum) {
if (me != NULL) {
if (likely(me != NULL)) {
H2K_runlist_remove(me);
H2K_ready_append(me);
if (unlikely(me->vmblock != NULL && me->vmblock->exiting)) {
/* VM is tearing down (a peer called H2K_vm_stop and IPI'd us
* via CLUSTER_RESCHED_INT): reap self instead of requeuing,
* and hand dosched a NULL "previous" so our context isn't saved. */
H2K_timer_cancel_withlock(me);
H2K_free_context_locked(me->vmblock, me);
H2K_vmblock_finalize_if_done_locked(me->vmblock);
me = NULL;
} else {
H2K_ready_append(me);
}
} else {
/* Interrupted WAIT mode */
H2K_gp->wait_mask = Q6_R_clrbit_RR(H2K_gp->wait_mask,hwtnum);
Expand Down
18 changes: 18 additions & 0 deletions kernel/thread/stop/stop.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,26 @@
#define H2K_STOP_H 1

#include <context.h>
#include <vmblock.h>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thread stop and vm stop should probably be separated (although perhaps one should call the other?)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that. Should have kept reading...


void H2K_thread_stop(s32_t status, H2K_thread_context *me) __attribute((noreturn)) IN_SECTION(".text.misc.stop");

/* POSIX exit() / process termination from any thread. Tears down the entire
* vmblock: reaps every non-DEAD context (including blocked, ready, vmwait,
* and via IPI any RUNNING peers), then signals the parent VM. */
void H2K_vm_stop(s32_t status, H2K_thread_context *me) __attribute((noreturn)) IN_SECTION(".text.misc.stop");

/* Signal the parent VM (if any) and free the vmblock if num_cpus has reached
* zero. Caller must hold BKL. Used by both thread_stop and the resched
* self-reap path to avoid leaking the vmblock when the last context exits. */
void H2K_vmblock_finalize_if_done_locked(H2K_vmblock_t *vmblock) IN_SECTION(".text.misc.stop");

/* Common teardown tail: drop the context's ASID ref, clear it (preserving
* vmblock_id), push it onto its vmblock's free list, and decrement num_cpus.
* Caller holds BKL and is responsible for the path-specific cancel/remove
* steps (timer/futex/intpool/ready/runlist) BEFORE calling this. */
void H2K_free_context_locked(H2K_vmblock_t *vmblock,
H2K_thread_context *ctx) IN_SECTION(".text.misc.stop");

#endif

164 changes: 147 additions & 17 deletions kernel/thread/stop/stop.ref.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,43 +9,173 @@
#include <thread.h>
#include <dosched.h>
#include <runlist.h>
#include <readylist.h>
#include <asid.h>
#include <stop.h>
#include <timer.h>
#include <vm.h>
#include <cpuint.h>
#include <id.h>
#include <alloc.h>
#include <futex.h>
#include <intpool.h>
#include <max.h>

void H2K_thread_stop(s32_t status, H2K_thread_context *me)
void H2K_vmblock_finalize_if_done_locked(H2K_vmblock_t *vmblock)
{
H2K_vmblock_t *vmblock = me->vmblock;
H2K_thread_context *parent_context;
H2K_vmblock_t *parent_vmblock;

if (vmblock->num_cpus != 0 && vmblock->status == 0) return;

parent_context = H2K_id_to_context(vmblock->parent);
if (parent_context != NULL
&& parent_context->status != H2K_STATUS_DEAD) {
parent_vmblock = parent_context->vmblock;
H2K_vm_cpuint_post_locked(parent_vmblock, parent_context, H2K_VM_CHILDINT, parent_vmblock->intinfo);
} else if (vmblock->num_cpus == 0) {
/* Can't free immediately because H2K_switch reads from *me */
/* EJP: I think this is OK now if we dosched(NULL,htnum)? */
H2K_mem_alloc_free(vmblock);
}
}

/* See stop.h. The five-step "return a context to its free list" tail shared
* by reap_one_locked, vm_stop_locked, H2K_thread_stop, and resched's
* self_reap_locked. */
void H2K_free_context_locked(H2K_vmblock_t *vmblock, H2K_thread_context *ctx)
{
H2K_asid_table_dec(ctx->ssr_asid);
H2K_thread_context_clear(ctx); /* preserves vmblock_id */
ctx->next = vmblock->free_threads;
vmblock->free_threads = ctx;
vmblock->num_cpus--;
}

/* Cancel pending waits and return one context to its vmblock's free list.
* Caller holds BKL. Decrements num_cpus on success.
* Skips H2K_STATUS_RUNNING contexts: those are executing on another HW thread
* and must self-reap via the exiting-vmblock path in resched. */
static int reap_one_locked(H2K_thread_context *ctx)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this code has the race condition issue that I ran into on my branch. Let's discuss.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this function seems to duplicate a lot with other thread cancel functionality, duplicating it is likely to cause problems over time with maintenance.

@andreykarpenko-qc andreykarpenko-qc Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`● I have the complete picture. Let me give you the full evidence-based investigation.

  Investigation: the "B stranded on a torn-down vmblock" claim

  I traced the +653 scenario end-to-end in the debugger and compared against the pre-fix master code. My original one-line characterization was directionally 
  right but imprecise. Here is what actually happens.

  What I measured (fixed kernel, +653)

  The reap sequence during A's exit(0) teardown, watched live:

  ┌───────────────────────────────────┬────────────────────────────┬───────────────────────────────────────────────────────────────────────────────────────┐
  │               event               │          context           │                                         path                                          │
  ├───────────────────────────────────┼────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────┤
  │ A frees itself                    │ me = 0xff015800            │ vm_stop_locked:108 (per-me cleanup)                                                   │
  ├───────────────────────────────────┼────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────┤
  │ A reaps parked main (BLOCKED on   │ 0xff015a80                 │ vm_stop_locked:113 → reap_one_locked → H2K_futex_cancel + H2K_free_context_locked     │
  │ main_park)                        │                            │                                                                                       │
  ├───────────────────────────────────┼────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────┤
  │ A scans B (still RUNNING)         │ 0xff015940                 │ reap_one_locked → case RUNNING: return 0 — skipped                                    │
  ├───────────────────────────────────┼────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────┤
  │ A's finalize                      │ vmblock 0xff013300         │ num_cpus=1, status=0, exiting=1 → guard (num_cpus!=0 && status==0) returns early —    │
  │                                   │                            │ vmblock NOT freed                                                                     │
  ├───────────────────────────────────┼────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────┤
  │ A releases BKL (via H2K_dosched)  │ —                          │ —                                                                                     │
  ├───────────────────────────────────┼────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────┤
  │ B acquires BKL, commits BLOCKED   │ 0xff015940, byte@+0xb:     │ futex_classic.S:87                                                                    │
  │                                   │ 0x01→0x03                  │                                                                                       │
  ├───────────────────────────────────┼────────────────────────────┼───────────────────────────────────────────────────────────────────────────────────────┤
  │ B self-reaps                      │ 0xff015940                 │ H2K_dosched → resched() sees me->vmblock->exiting → frees self, clears context (ring  │
  │                                   │                            │ node → self-referential)                                                              │
  └───────────────────────────────────┴────────────────────────────┴───────────────────────────────────────────────────────────────────────────────────────┘

  The critical measured fact: at A's finalize, num_cpus == 1. That single outstanding count is B. The finalize guard therefore does not free the vmblock — B's
  un-reaped slot keeps it alive. This is exactly what makes B's later BLOCKED commit safe: the vmblock is still valid when B touches me->vmblock->exiting.

  The two mechanisms that catch B (this branch adds both)

  1. vmblock->exiting gate in resched() (resched.ref.c:22-29) — new on this branch. When B finally yields via H2K_dosched, resched sees exiting=1 and self-reaps
  B instead of requeuing it, then runs finalize_if_done which now sees num_cpus==0 and completes teardown.
  2. Pass-2 IPI in vm_stop_locked — A fires CLUSTER_RESCHED_INT at every RUNNING peer's HW thread, forcing B into H2K_resched_cluster → same exiting self-reap
  path even if B never voluntarily blocks.

  Either path converges on the same self-reap. I confirmed the VM ends fully torn down (all HW threads in H2K_wait_forever, B's context recycled to the free
  list).

  What actually happens on the pre-fix kernel (master)

  I checked — and my "use-after-free of the vmblock" wording was too strong. On master:
  - There is no H2K_vm_stop, no exiting field, no reap_one_locked, no Pass-2 IPI at all. The whole VM-teardown-on-exit path is new on this branch.
  - resched() on master unconditionally does H2K_ready_append(me); H2K_dosched(me) — no exiting-gate.

  So the real pre-fix failure mode isn't "B dereferences a freed vmblock." It's that exit() had no whole-VM teardown mechanism: a peer blocking on a futex
  during another thread's exit would either be stranded as a permanently BLOCKED thread that's never reaped (missed-wakeup / leaked context — the futex is never
  resumed, no IPI ever comes), or the teardown accounting is simply absent. The UAF risk is the design hazard this branch had to avoid while adding teardown —
  i.e., if teardown freed the vmblock while B was still RUNNING and about to block, B would then touch freed memory. The num_cpus==1 early-return I measured is
  precisely the guard that prevents that.

  Corrected statement

  ▎ B commits to BLOCKED after A's reap scan skips it (B still RUNNING). The fix keeps the vmblock alive across this window (A's finalize early-returns because
  ▎ num_cpus==1 accounts for un-reaped B), and guarantees B is subsequently reaped via the new vmblock->exiting self-reap path in resched — reached either when 
  ▎ B voluntarily blocks or via A's Pass-2 CLUSTER_RESCHED_INT. Without this branch's teardown machinery, that late-blocking peer had no reaper at all: it would
  ▎ leak as a stranded BLOCKED context on a VM that's supposed to be gone.
 `

{
H2K_vmblock_t *vmblock = ctx->vmblock;
u8_t s = ctx->status;

switch (s) {
case H2K_STATUS_DEAD:
case H2K_STATUS_RUNNING:
return 0;
case H2K_STATUS_BLOCKED:
H2K_timer_cancel_withlock(ctx);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be moved to be common?

H2K_futex_cancel(ctx);
break;
case H2K_STATUS_INTBLOCKED:
H2K_timer_cancel_withlock(ctx);
/* TODO: H2K_popup_wait shares INTBLOCKED with H2K_intpool_wait;
* H2K_intpool_cancel only handles the latter. Distinguish before
* canceling once popup_wait grows a state bit. */
H2K_intpool_cancel(ctx);
break;
case H2K_STATUS_READY:
H2K_ready_remove(ctx);
H2K_timer_cancel_withlock(ctx);
break;
case H2K_STATUS_VMWAIT:
if (ctx->id.cpuidx < (sizeof(long_bitmask_t) * 8))
vmblock->waiting_cpus &= ~(0x1ULL << ctx->id.cpuidx);
H2K_timer_cancel_withlock(ctx);
break;
default:
return 0;
}
H2K_free_context_locked(vmblock, ctx);
return 1;
}

/* Tear down the calling thread's entire vmblock: reap every non-DEAD context,
* IPI any RUNNING peers so they self-reap, and run the parent-signal /
* vmblock-free finalizer. Caller holds BKL. POSIX exit() semantics. */
static void vm_stop_locked(s32_t status, H2K_thread_context *me)
{
H2K_vmblock_t *vmblock = me->vmblock;
u32_t i;

vmblock->exiting = 1;

/* Per-me cleanup. */
H2K_timer_cancel_withlock(me);
H2K_runlist_remove(me);
H2K_free_context_locked(vmblock, me);
vmblock->status = status;

/* Pass 1: reap every non-DEAD, non-RUNNING context immediately. */
for (i = 0; i < vmblock->max_cpus; i++) {
reap_one_locked(&vmblock->contexts[i]);
}
/* Pass 2: kick any RUNNING contexts via CLUSTER_RESCHED_INT steered to
* their HW thread. Each target enters H2K_resched_cluster, sees
* vmblock->exiting and self-reaps. */
for (i = 0; i < vmblock->max_cpus; i++) {
H2K_thread_context *ctx = &vmblock->contexts[i];
if (ctx->status != H2K_STATUS_RUNNING) continue;
if (ctx == me) continue;
iassignw(CLUSTER_RESCHED_INT, ~(0x1u << ctx->hthread));
cluster_resched_int();
}

H2K_vmblock_finalize_if_done_locked(vmblock);
}

void H2K_thread_stop(s32_t status, H2K_thread_context *me)
{
H2K_vmblock_t *vmblock = me->vmblock;
u32_t i;

BKL_LOCK(&H2K_bkl);

/*
* Per-thread cleanup. Main and worker threads share this path:
* pthread_exit on main must NOT tear down the VM (POSIX semantics --
* workers keep running). VM-wide teardown happens only via
* H2K_vm_stop (H2_TRAP_VM_STOP), which exit()/sys_exit() invokes.
*/
H2K_timer_cancel_withlock(me);
H2K_runlist_remove(me);
H2K_asid_table_dec(me->ssr_asid);
H2K_thread_context_clear(me);
me->next = vmblock->free_threads;
vmblock->free_threads = me;
vmblock->num_cpus--;
H2K_free_context_locked(vmblock, me);
vmblock->status = status;

if (status != 0 || vmblock->num_cpus == 0) { // signal parent
parent_context = H2K_id_to_context(vmblock->parent);
if (parent_context != NULL
&& parent_context->status != H2K_STATUS_DEAD) { // parent exists
parent_vmblock = parent_context->vmblock;
H2K_vm_cpuint_post_locked(parent_vmblock, parent_context, H2K_VM_CHILDINT, parent_vmblock->intinfo);
} else if (vmblock->num_cpus == 0) { // no parent; just deallocate.
/* Can't free immediately because H2K_switch reads from *me */
/* EJP: I think this is OK now if we dosched(NULL,htnum)? */
H2K_mem_alloc_free(vmblock);
if (!vmblock->exiting && status == 0 && vmblock->num_cpus > 0) {
/* A thread exited cleanly with siblings remaining (main and
* workers alike): keep the conservative all-blocked-with-no-
* armed-timer reaper. A non-zero ctx->timeout means a timer is
* queued; that thread will be woken when it fires, so do not reap. */
int all_blocked = 1;
for (i = 0; i < vmblock->max_cpus && all_blocked; i++) {
H2K_thread_context *ctx = &vmblock->contexts[i];
u8_t s = ctx->status;
if (s == H2K_STATUS_DEAD)
continue;
if ((s == H2K_STATUS_BLOCKED || s == H2K_STATUS_INTBLOCKED) && ctx->timeout == 0)
continue;
all_blocked = 0;
}
if (all_blocked) {
for (i = 0; i < vmblock->max_cpus; i++) {
reap_one_locked(&vmblock->contexts[i]);
}
}
}

H2K_vmblock_finalize_if_done_locked(vmblock);

/* If we dosched(NULL,get_hwtnum()) I think we can remove special cases in free() */
H2K_dosched(NULL,get_hwtnum());
}

void H2K_vm_stop(s32_t status, H2K_thread_context *me)
{
BKL_LOCK(&H2K_bkl);
vm_stop_locked(status, me);
H2K_dosched(NULL, get_hwtnum());
/* unreachable */
}
2 changes: 1 addition & 1 deletion libs/h2/h2/h2_trap_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#define H2_TRAP_THREAD_CREATE 4
#define H2_TRAP_THREAD_STOP 5
#define H2_TRAP_CPUTIME 6
//#define H2_UNUSED 7
#define H2_TRAP_VM_STOP 7
#define H2_TRAP_REGISTER_FASTINT 8
#define H2_TRAP_PRIO_SET 9
#define H2_TRAP_PRIO_GET 10
Expand Down
10 changes: 10 additions & 0 deletions libs/h2/thread/h2_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ Terminate the current thread with the given status.
*/
void h2_thread_stop_trap(int status);

/**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows the process to tear itself down, but doesn't allow you to kill a child VM... that's also a useful thing to be able to do.

Tear down the entire VM with the given exit status (POSIX exit() semantics).
All other contexts in the calling thread's vmblock are reaped regardless of
state, then the parent VM is signaled. Caller does not return.
@param[in] status Termination status value
@returns None; Does not return.
@dependencies None
*/
void h2_vm_stop_trap(int status) __attribute__((noreturn));

/**
Obtain the ID of the calling thread
@returns ID of the calling thread
Expand Down
5 changes: 5 additions & 0 deletions libs/h2/thread/h2_thread.ref.S
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ FUNC_START h2_thread_stop_trap .text.h2.thread .p2align 2
jumpr r31
FUNC_END h2_thread_stop_trap

FUNC_START h2_vm_stop_trap .text.h2.thread .p2align 2
trap0(#H2_TRAP_VM_STOP)
jumpr r31
FUNC_END h2_vm_stop_trap

FUNC_START h2_yield .text.h2.thread .p2align 2
trap0(#H2_TRAP_YIELD)
jumpr r31
Expand Down
Loading
Loading