feat(ipc/sem): add semaphore support for dragonOS by mistcoversmyeyes · Pull Request #2172 · DragonOS-Community/DragonOS

mistcoversmyeyes · 2026-08-08T11:14:05Z

Summary

Implement the x86_64 System V semaphore syscalls: semget, semctl, semop, and semtimedop.
Add semaphore-set management to IPC namespaces.
Extract and reuse common System V IPC permission checks.

Scope

Match the Linux 6.6 x86_64 ABI and observable behavior.
SEM_UNDO is out of scope and currently returns ENOSYS.

Acceptance

Valid semaphore syscall requests no longer return ENOSYS.
semop and semtimedop share consistent operation semantics.
Creation, lookup, control, removal, and permission checks match Linux behavior.
Multi-operation requests execute atomically.
Blocking operations wake correctly after value changes or IPC_RMID.
Nonblocking, timeout, signal, invalid-argument, and removed-set errors match Linux behavior.
Concurrent access avoids races, lost wake-ups, use-after-free, and resource leaks.
Existing DragonOS CI tests pass.

Testing

Added 43 System V semaphore dunitests.
QEMU guest test: 43/43 passed.
Format, Clippy, multi-architecture builds, Dunitest, and Integration Test CI passed.

fslongjin · 2026-08-29T09:54:18Z

@codex review

chatgpt-codex-connector · 2026-08-29T09:54:37Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-29T10:00:44.439894Z	`fbdee92`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fbdee927ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-29T10:00:41Z

+        let set = self
+            .get_by_semid_checked_mut(token.id)
+            .map_err(|_| SystemError::EIDRM)?;


在 SETALL 提交前重新检查写权限

当 prepare_setall() 检查权限后复制用户数组时，集合所有者可并发执行 IPC_SET 撤销调用者的写权限；这里重新加锁后只验证 ID 和长度，仍会提交全部新值。应在持锁修改 semval 前按当前权限再次执行写权限检查，避免权限撤销后的 TOCTOU 写入。

AGENTS.md reference: AGENTS.md:L9-L9

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-29T10:00:41Z

+        self.uid = make_kuid(user_ns, uid)?;
+        self.gid = make_kgid(user_ns, gid)?;


限制非特权 IPC_SET 的所有者变更

当信号量集的非特权所有者或创建者执行 IPC_SET 时，check_control_permission() 会放行，而该赋值逻辑接受任意可映射的 UID/GID，允许调用者把对象归属伪造成其他用户或自己不属于的组。Linux 仅允许无相应 capability 的调用者使用自身身份或所属组，因此应在写入前验证目标 UID/GID。

AGENTS.md reference: AGENTS.md:L9-L9

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-29T10:00:41Z

+        self.uid = make_kuid(user_ns, uid)?;
+        self.gid = make_kgid(user_ns, gid)?;


在映射全部 IPC_SET 身份后再写入

在用户命名空间中提交可映射的 UID 和不可映射的 GID 时，第一个赋值已经修改对象所有者，随后 make_kgid() 返回 EINVAL；因此失败的 IPC_SET 会留下部分更新，调用者甚至可能意外失去对象控制权。应先把两个映射结果保存到局部变量，全部成功后再一次性更新权限对象。

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-29T10:00:41Z

+    }
+
+    fn get_by_index(&self, id: usize) -> Result<&KernelSemSet, SystemError> {
+        let idx = id & IpcIdAllocator::IPC_ID_IDX_MASK;


拒绝超出范围的 SEM_STAT 索引

当 SEM_STAT 或 SEM_STAT_ANY 收到大于 IPC_ID_IDX_MASK 的索引时，这里静默截掉高位；只要低 15 位对应现有集合，诸如 0x8000 的无效索引就会错误返回索引 0 的对象，而不是 Linux 的 EINVAL。应先验证范围，再直接按原索引查表。

AGENTS.md reference: AGENTS.md:L9-L9

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-29T10:00:41Z

+                        if changed {
+                            break;


避免每完成一个等待者就全量重扫队列

当队首有 B 个仍不可执行的等待者、其后有 W 个依次可执行且改变值的等待者时，这个 break 会在每次完成后从队首重新扫描；每次模拟还会分配 HashMap，并且全过程持有命名空间级自旋锁，形成 O(B×W) 的模拟和分配。等待者数量不受 SEMOPM 限制，非特权进程可借此造成整个 IPC 命名空间长时间停顿，应仅重新调度受值变化影响的条目或使用索引化等待队列。

AGENTS.md reference: AGENTS.md:L10-L10

Useful? React with 👍 / 👎.

fslongjin

Request changes: this PR establishes a useful base for System V semaphore support, but it does not yet satisfy the Linux 6.6 compatibility and concurrency-safety contract stated in #2142.

The blocking issues are:

IPC_SET permission updates can partially commit on an error, changing the owner even though the syscall returns EINVAL; the shared helper also affects SHM.
SEM_UNDO is rejected with ENOSYS and the new test codifies that incompatibility, while Linux maintains per-process/shared undo state and replays it at process exit.
A single namespace-wide spinlock protects the registry and every semaphore set, so unrelated sets are serialized; the lock also covers allocation-heavy queue simulation and scheduler wakeups.
User-controlled semaphore-set allocation is infallible and can reach the kernel panic allocation handler instead of returning ENOMEM.
SEM_STAT and SEM_STAT_ANY mask their direct table index, causing out-of-range indices to alias valid objects.

The basic syscall wiring, atomic multi-operation simulation, timeout/removal paths, and test breadth are valuable. However, the issues above are architectural or user-visible Linux semantic mismatches rather than optional refinements. Please address them, add the corresponding regression tests, and rerun the guest suite. The current Integration Test check also reports 5666 passed, 1 failed, and 180 skipped; I am not attributing that failure to this PR without further evidence, but the PR description should not claim that Integration Test passed while the check remains red.

fslongjin · 2026-08-29T16:15:32Z

+        user_ns: &Arc<UserNamespace>,
+    ) -> Result<(), SystemError> {
+        self.uid = make_kuid(user_ns, uid)?;
+        self.gid = make_kgid(user_ns, gid)?;


[P1] Make IPC_SET permission updates transactional. self.uid is mutated before make_kgid() can fail, so a request with a mappable UID and an unmappable GID returns EINVAL after permanently changing the object's owner. Linux 6.6 ipc_update_perm() converts and validates both IDs into local variables before committing either field. Please compute new_uid and new_gid first, then update uid/gid/mode together. Because this helper is shared, the current error path also affects SHM.

fslongjin · 2026-08-29T16:15:32Z

+            .iter()
+            .any(|op| (op.sem_flg as u32) & SemFlags::SEM_UNDO.bits() != 0)
+        {
+            return Err(SystemError::ENOSYS);


[P1] Rejecting every SEM_UNDO operation with ENOSYS is not Linux-compatible System V semaphore behavior. Linux 6.6 maintains sem_undo/semadj state, shares the undo list for CLONE_SYSVSEM, clears adjustments on SETVAL/SETALL/IPC_RMID, and replays them from exit_sem() when a task exits. This is essential crash-recovery behavior: without it, a lock holder exiting can leave peers blocked indefinitely. Please implement the full lifecycle before treating #2142 as complete; the new test should verify Linux behavior instead of expecting ENOSYS.

fslongjin · 2026-08-29T16:15:32Z

+    pub fn new(kern_ipc_perm: IpcPerm, nsems: usize) -> Self {
+        KernelSemSet {
+            kern_ipc_perm,
+            sems: core::iter::repeat_n(KernelSem { val: 0, pid: None }, nsems).collect(),


[P1] This allocation must be fallible. nsems is user-controlled up to 32,000, while collect() uses the global infallible allocation path; DragonOS's alloc_error_handler panics on failure. Linux's newary() returns ENOMEM when sem_alloc() fails. Please use try_reserve_exact() (or an equivalent fallible constructor), return ENOMEM, and roll back the IPC ID allocated immediately before this constructor if allocation fails.

fslongjin · 2026-08-29T16:15:32Z

+    }
+
+    fn get_by_index(&self, id: usize) -> Result<&KernelSemSet, SystemError> {
+        let idx = id & IpcIdAllocator::IPC_ID_IDX_MASK;


[P1] SEM_STAT and SEM_STAT_ANY take a direct IPC table index, not an encoded semaphore ID. Masking the argument makes an invalid index such as 0x8000 + n alias slot n and return a valid set, whereas Linux passes the original integer to the IDR lookup and returns EINVAL. Please reject indices above IPC_ID_IDX_MASK and look up the unmodified index; add coverage for both commands with high-bit indices.

fslongjin · 2026-08-29T16:15:32Z

+    /// SysV SHM manager (phase one: per-namespace SHM only)
    pub shm: SpinLock<ShmManager>,
+    /// SysV semaphore manager
+    pub sem: SpinLock<SemManager>,


[P1] A namespace-wide spinlock is too broad for semaphore-set state. Every operation on every set, including update_queue(), is serialized here; queue simulation allocates a HashMap, may rescan waiters quadratically, and calls Waker::wake() while this lock is held. A user can therefore stall unrelated semaphore sets in the same namespace. Please keep the manager lock limited to ID/key/quota lookup, store stable Arc<KernelSemSet> objects with per-set locking, use a non-allocating operation fast path, and collect wakeups for execution after releasing the set lock, following Linux's registry/array locking and wake_q separation.

github-actions Bot added the enhancement New feature or request label Aug 8, 2026

fslongjin reviewed Aug 9, 2026

View reviewed changes

Comment thread kernel/src/ipc/syscall/sys_semop.rs Outdated

mistcoversmyeyes force-pushed the feat/ipc-sem-2142 branch 2 times, most recently from e0bc761 to a266b20 Compare August 17, 2026 09:07

github-actions Bot added the test Unitest/User space test label Aug 19, 2026

mistcoversmyeyes marked this pull request as ready for review August 19, 2026 07:43

mistcoversmyeyes added 5 commits August 26, 2026 19:28

refactor(ipc): extract generic SysV IPC permission module

d025357

feat(ipc): implement System V semaphore syscalls

8535058

test(ipc): add SysV semaphore dunitest suite

60fba22

style(ipc): satisfy kernel formatting checks

74879fd

style(ipc): translate SysV semaphore comments to English

fbdee92

mistcoversmyeyes force-pushed the feat/ipc-sem-2142 branch from d9c382f to fbdee92 Compare August 26, 2026 11:29

chatgpt-codex-connector Bot reviewed Aug 29, 2026

View reviewed changes

fslongjin requested changes Aug 29, 2026

View reviewed changes

		self.uid = make_kuid(user_ns, uid)?;
		self.gid = make_kgid(user_ns, gid)?;

Uh oh!

Uh oh!

Conversation

mistcoversmyeyes commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related

Summary

Scope

Acceptance

Testing

Uh oh!

Uh oh!

fslongjin commented Aug 29, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin left a comment

Choose a reason for hiding this comment

Uh oh!

fslongjin Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

fslongjin Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mistcoversmyeyes commented Aug 8, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Aug 29, 2026 •

edited

Loading