Repository navigation
Conversation
Give every socket inode a sockfs identity and per-socket mutable metadata so the generic VFS paths can implement fstatfs(2), fchmod(2), and fstat(2) without socket-specific syscall branches. - Reuse a singleton SockFs for SOCKFS_MAGIC, PAGE_SIZE, NAME_MAX and frsize. - Initialize sockets with S_IFSOCK | 0777 and creator fsuid/fsgid. - Implement masked metadata updates so do_fchmod keeps owner, CAP_FSETID, immutable, SGID and ctime semantics in one place. - Derive Unix path bind permissions from the socket mode and current umask, matching Linux unix_bind_bsd(). - Keep AF_PACKET's mmap filesystem while sharing the sockfs statfs view. - Cover the behavior with dunitest and unblock the three gVisor cases. Signed-off-by: longjin <longjin@dragonos.org>
github-actions
Bot
added
Bug fix
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66a5dfe51a
ℹ️ 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".
Linux allocates the accepted socket's sockfs inode in the accepting task (`sock_alloc()` from `do_accept()`), so `i_uid`/`i_gid` come from the acceptor's fsuid/fsgid. DragonOS materializes some passive peers before accept: Unix stream children are created by the connecting task in `connect()`, and vsock children may be created by a transport worker. They therefore inherited the wrong credentials, so `fstat(2)` reported the peer's owner and an unprivileged acceptor could fail `fchmod(2)` with EPERM. Refresh the accepted socket's metadata owner in the shared accept path before publishing the descriptor. The reset is protocol-agnostic, so it covers Unix stream and vsock children without touching their handoff paths. Add a dunitest that connects as one euid, accepts from root, and checks the accepted inode ownership, and remove an explicit auto-deref flagged by clippy. Verification: - make kernel - make fmt FMT_CHECK=--check - QEMU dunitest normal/unix_socket_stat: 6/6 passed Signed-off-by: longjin <longjin@dragonos.org>
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
CI status on
So the only red check is the pre-existing exec failure on master; this PR's socket behavior is covered by Dunitest and the target gVisor cases. |
The gVisor exec tests resolve /bin/echo and /bin/bash directly. The default CI rootfs is FAT32 built from BusyBox, which only installs /bin/busybox and /bin/sh because FAT cannot represent applet symlinks. The four unblocked tests therefore failed with ENOENT even though the exec and namei semantics were correct. Enable the BusyBox bash applet in both the DADK and Nix rootfs builds, and materialize the two interpreter entry points at test start only when they are missing. An existing executable, such as a base image's real bash, is never replaced. Fixture setup now fails fast with a precise diagnostic instead of letting the tests degrade into ENOENT. Verified on a clean default FAT32 image: the fixture installs both entry points, the four previously failing exec tests pass, and the socket statfs/permission cases remain green. Signed-off-by: longjin <longjin@dragonos.org>
|
The Integration Test failure was not only pre-existing: the four exec tests fail because the default CI rootfs is a FAT32 image built from BusyBox, which installs only Fixed in
Verified on a freshly rebuilt
|
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
CI on |
Summary
Make socket inodes participate in the normal VFS metadata and statfs contracts, so Unix sockets no longer need syscall-specific branches.
sockfsidentity and per-socket mutable inode metadata.SOCKFS_MAGIC,PAGE_SIZE,NAME_MAXandfrsize == bsizefromfstatfs(2).S_IFSOCK | 0777and the creator'sfsuid/fsgid, matching Linuxsock_alloc().do_fchmod()through masked metadata updates, preserving owner/CAP_FSETID/immutable/SGID/ctime semantics.unix_bind_bsd().Verification
make kernel: passed.SocketTest.UnixSocketStat,SocketTest.UnixSocketStatFS,SocketTest.Permission: 3/3 passed in QEMU (KVM, snapshot disk).normal/unix_socket_stat: 5/5 passed in QEMU.AfPacketRing.FstatfsUsesSockfsBeforeAndAfterRingSetup: passed in QEMU.No smoltcp change was required: this fix is confined to socket inode identity/metadata and path bind semantics.