feat: musl libc full support - syscalls, env, pthread, epoll, and test programs#230
feat: musl libc full support - syscalls, env, pthread, epoll, and test programs#230
Conversation
… and test programs Add kernel syscalls (getrlimit/prlimit64, uname, epoll_create1/ctl/wait/pwait), environment variable support (envp stack parsing, getenv), real pthread mutexes/condvars via futex, and libbreenix-libc wrappers. Includes three new musl C test programs (env_musl_test, uname_musl_test, rlimit_musl_test) that exercise these syscalls end-to-end through stock musl libc. Co-Authored-By: Ryan Breen <ryanbreen@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 6 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on March 12
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| /* Test 2: getenv("HOME") returns "/home" */ | ||
| val = getenv("HOME"); | ||
| if (val != NULL && strcmp(val, "/home") == 0) { |
There was a problem hiding this comment.
Test expects HOME=/home but kernel sets HOME=/
Medium Severity
The env_test.c Test 2 checks strcmp(val, "/home") == 0 for getenv("HOME"), but all three default_env definitions in process/manager.rs set HOME=/ (i.e. b"HOME=/\0"). This mismatch means Test 2 will always fail, and since the test returns 1 on any failure, the entire env_musl_test program will report failure.
Additional Locations (1)
| FdKind::Epoll(id) => { | ||
| // Clean up the epoll instance | ||
| crate::syscall::epoll::remove_instance(id); | ||
| } |
There was a problem hiding this comment.
Epoll instance removed prematurely after fork
Medium Severity
FdTable::clone() (used during fork) copies FdKind::Epoll(id) without incrementing any reference count (falls through to the _ => {} arm). Both parent and child point to the same global epoll instance. When either process closes the epoll fd or exits, remove_instance(id) unconditionally removes the shared instance from the global registry, leaving the other process with a dangling epoll reference that returns EBADF.
Additional Locations (1)
| b"TERM=vt100\0", | ||
| b"USER=root\0", | ||
| b"SHELL=/bin/bsh\0", | ||
| ]; |
There was a problem hiding this comment.
Triplicated default_env array across process creation paths
Low Severity
The identical default_env array (5 entries, same values) is defined three separate times in process/manager.rs — once in create_process_with_argv and twice in different exec_process_with_argv variants. A single shared constant would avoid the risk of inconsistent updates (like the HOME bug) and reduce maintenance burden.
Additional Locations (2)
| let thread_id = match crate::task::scheduler::current_thread_id() { | ||
| Some(id) => id, | ||
| None => return SyscallResult::Err(errno::ESRCH as u64), | ||
| }; |
There was a problem hiding this comment.
Epoll instance leaked on early error path
Low Severity
In sys_epoll_create1, alloc_instance() is called on line 145, adding an entry to the global EPOLL_INSTANCES registry. If current_thread_id() returns None on line 148, the function returns an error without calling remove_instance(epoll_id), permanently leaking the allocated epoll instance. The other two error paths (lines 157 and 165) correctly call remove_instance, but this first one was missed.
| FdKind::ProcfsDirectory { .. } => { | ||
| // Procfs directory doesn't need cleanup | ||
| } | ||
| FdKind::Epoll(id) => { |
There was a problem hiding this comment.
Epoll instance leaked during exec close_cloexec cleanup
Medium Severity
When close_cloexec runs during exec(), epoll fds marked with FD_CLOEXEC are removed from the fd table but fall through to the _ => {} catch-all without calling remove_instance. This permanently leaks the epoll instance in the global EPOLL_INSTANCES registry. This is especially likely since EPOLL_CLOEXEC is the standard flag passed to epoll_create1.
| FdKind::FifoWrite(path, _) => write!(f, "FifoWrite({})", path), | ||
| FdKind::ProcfsFile { content, position } => write!(f, "ProcfsFile(len={}, pos={})", content.len(), position), | ||
| FdKind::ProcfsDirectory { path, position } => write!(f, "ProcfsDirectory(path={}, pos={})", path, position), | ||
| FdKind::Epoll(id) => write!(f, "Epoll({})", id), |
There was a problem hiding this comment.
Epoll instance leaked when dup2 overwrites epoll fd
Low Severity
When dup2 (or dup3) overwrites a new_fd that is an epoll fd, the old entry is taken from the table and its kind is matched for cleanup. FdKind::Epoll falls through to _ => {} without calling remove_instance, leaking the epoll instance in the global EPOLL_INSTANCES registry.
1 participant
Summary
getenv()andenvironwork out of the boxenv_musl_test,uname_musl_test,rlimit_musl_testexercising the new syscalls end-to-end through stock musl libcTest plan
make && make installinuserspace/c-programs/builds all 4 musl programs cleanlyrun-aarch64-boot-test-native.shpasses — new test programs print PASS for all checks🤖 Generated with Claude Code
Note
High Risk
High risk because it adds new kernel syscalls (including
epoll) with new FD kinds/global instance tracking and changes initial process stack/env handling plus futex-backed pthread primitives, all of which affect core userspace compatibility and concurrency behavior.Overview
Adds Linux-compatible syscalls for resource/system info (
getrlimit/prlimit64,uname) andepoll(epoll_create1,epoll_ctl,epoll_wait/epoll_pwait) across both x86_64 and ARM64 dispatch paths, including a newFdKind::Epollwith close/drop cleanup and basicfstatbehavior.Updates process startup stack construction to include a default
envp, and updateslibbreenix-libcstartup +getenv()to parse/useenviron. Also replaces pthread mutex/condvar stubs with futex-based implementations and adds libc wrappers forepoll,uname, andgetrlimit.Extends ARM64 musl test coverage by adding/building/installing new C test programs (
env_musl_test,uname_musl_test,rlimit_musl_test) and including them in the boot test list.Written by Cursor Bugbot for commit b211cd3. This will update automatically on new commits. Configure here.