Skip to content

Add U-mode task isolation and fix self-termination#76

Open
HeatCrab wants to merge 1 commit intosysprog21:mainfrom
HeatCrab:u-mode/syscall-enhancement
Open

Add U-mode task isolation and fix self-termination#76
HeatCrab wants to merge 1 commit intosysprog21:mainfrom
HeatCrab:u-mode/syscall-enhancement

Conversation

@HeatCrab
Copy link
Collaborator

@HeatCrab HeatCrab commented Feb 2, 2026

U-mode tasks could previously control other tasks and had no way to properly terminate themselves. This adds permission checks to restrict task control syscalls to self-only operations and enables safe self-termination through the existing zombie task mechanism.


Summary by cubic

Locks down U‑mode task control to self-only and adds safe self-termination via the zombie task path. This prevents user tasks from affecting others and ensures proper cleanup.

  • Bug Fixes
    • U‑mode isolation and ID checks: tcancel/tsuspend/tpriority require id in 1..UINT16_MAX and equal to mo_task_id(); out-of-range ids return -EINVAL; others return -EPERM. tresume always returns -EPERM in U‑mode.
    • Safe self-termination: mo_task_cancel(self) sets TASK_ZOMBIE and yields; the scheduler reclaims it later. Zombie cleanup skips the current task; id == 0 still returns ERR_TASK_CANT_REMOVE.

Written for commit 8d968ab. Summary will update on new commits.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="kernel/task.c">

<violation number="1" location="kernel/task.c:1003">
P1: Marking the current task as TASK_ZOMBIE before _yield() lets dispatch() free the running task in task_cleanup_zombies(), then dereference kcb->task_current->data during context save/scheduling. This introduces a use-after-free on self-termination. Defer freeing the current task until after the context switch or skip cleanup of the current task.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@HeatCrab HeatCrab force-pushed the u-mode/syscall-enhancement branch from 8f6f132 to ad15807 Compare February 3, 2026 11:24
kernel/syscall.c Outdated
{
if (unlikely(id <= 0))
return -EINVAL;
if (id <= 0 || (uint16_t) id != mo_task_id())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the syscall interface takes int id (32-bit), but mo_task_id() returns uint16_t, there is a potential aliasing issue here due to the cast.

If a user accidentally passes a large ID like 0x10001 (65537), the check (uint16_t)0x10001 != 1 becomes 1 != 1 (False), allowing the call to proceed. Then, mo_task_cancel(uint16_t id) will receive the truncated value 1 and terminate the current task unexpectedly.

I think it would be safer to enforce an upper bound check or perform the comparison without the cast?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed. Adopted the upper bound check approach and preserved Linmo's original error code semantics.

@HeatCrab HeatCrab force-pushed the u-mode/syscall-enhancement branch from ad15807 to b9d44fd Compare February 3, 2026 15:22
static int _tsuspend(int id)
{
if (unlikely(id <= 0))
if (unlikely(id <= 0 || id > (int) UINT16_MAX))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I don't think we need the (int) cast here due to integer promotion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To satisfy -Wsign-compare without the (int) cast, adopted the single-check idiom.

Copy link
Collaborator

Choose a reason for hiding this comment

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

How about we keep the id > (int) UINT16_MAX check?
I understand the logic behind the (unsigned int) id - 1 trick, but it makes the code unnecessarily harder to read at a glance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure! Reverted to the explicit two-check form.

@HeatCrab HeatCrab force-pushed the u-mode/syscall-enhancement branch 2 times, most recently from f76afa7 to d34b392 Compare February 4, 2026 13:59
@HeatCrab HeatCrab requested a review from visitorckw February 4, 2026 14:06
U-mode tasks could previously control other tasks and had no way to
properly terminate themselves. This adds permission checks to restrict
task control syscalls to self-only operations and enables safe
self-termination through the existing zombie task mechanism.
@HeatCrab HeatCrab force-pushed the u-mode/syscall-enhancement branch from d34b392 to 8d968ab Compare February 13, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants