Add SLIRP user-mode networking (outbound TCP/UDP) - #3
Merged
Merged
Conversation
There was a problem hiding this comment.
6 issues found across 21 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Makefile">
<violation number="1" location="Makefile:142">
P2: Avoid running `fetch-minislirp.sh` via `$(shell ...)` at parse time; it causes side effects for unrelated targets like `clean` when SLIRP is enabled.</violation>
</file>
<file name="scripts/fetch-minislirp.sh">
<violation number="1" location="scripts/fetch-minislirp.sh:20">
P1: Validate `SLIRP_DIR` before `rm -rf` to avoid deleting unintended paths when the override is misconfigured.</violation>
</file>
<file name="src/seccomp-dispatch.c">
<violation number="1" location="src/seccomp-dispatch.c:868">
P2: FD leak when ADDFD result doesn't satisfy minfd. Once `kbox_notify_addfd` succeeds, the FD is already injected into the tracee. Setting `nh = -1` and falling through to CONTINUE causes the host kernel to create a second dup, leaving the ADDFD FD orphaned. Consider either not calling ADDFD when minfd > 0 and relying on CONTINUE, or closing the orphaned tracee FD before returning CONTINUE.</violation>
</file>
<file name="src/fd-table.c">
<violation number="1" location="src/fd-table.c:143">
P1: `kbox_fd_table_remove` may close unrelated supervisor FDs by treating tracee ADDFD numbers as local `host_fd`s.</violation>
<violation number="2" location="src/fd-table.c:216">
P1: CLOEXEC cleanup closes `host_fd` as if local, but it is a tracee ADDFD number and can corrupt supervisor FD state.</violation>
</file>
<file name="src/net-slirp.c">
<violation number="1" location="src/net-slirp.c:224">
P2: The `cmd_pipe` infrastructure is created and polled but never written to. Both `kbox_net_register_socket()` and `kbox_net_deregister_socket()` perform synchronous operations under `shadow_lock` instead of using the command pipe. This leaves `struct net_cmd`, `NET_CMD_REGISTER`, `NET_CMD_DEREGISTER`, and `process_cmd_pipe()` as dead code.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| fi | ||
|
|
||
| echo "Fetching minislirp from ${REPO} ..." | ||
| rm -rf "${SLIRP_DIR}" |
There was a problem hiding this comment.
P1: Validate SLIRP_DIR before rm -rf to avoid deleting unintended paths when the override is misconfigured.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/fetch-minislirp.sh, line 20:
<comment>Validate `SLIRP_DIR` before `rm -rf` to avoid deleting unintended paths when the override is misconfigured.</comment>
<file context>
@@ -0,0 +1,26 @@
+fi
+
+echo "Fetching minislirp from ${REPO} ..."
+rm -rf "${SLIRP_DIR}"
+git clone --depth=1 "${REPO}" "${SLIRP_DIR}"
+
</file context>
Bridge LKL's virtio-net device with minislirp to provide outbound networking without privileges. Guest processes can resolve DNS and make HTTP connections through the host network via --net. Core design: - LKL virtio-net bridged to SLIRP via length-prefixed TX/RX pipes - RX packet queue (SPSC ring buffer) between rx_reader thread and LKL net_rx callback; returns -1 when empty to prevent kworker refill spin that would starve lkl_cpu_get callers - Shadow sockets for INET SOCK_STREAM/SOCK_DGRAM: AF_UNIX socketpair injected into tracee via ADDFD, event loop pumps data to LKL socket - Non-INET, SOCK_RAW, and no-SLIRP sockets use virtual FD path - Interface configured via ioctl (SIOCSIFADDR creates connected routes) - net_poll gated on net_ready flag during boot/config - Registration and deregistration both synchronous under shadow_lock Socket dispatch additions: - forward_socket, forward_bind, forward_connect, forward_sendto, forward_recvfrom, forward_recvmsg, forward_getsockopt, forward_setsockopt, forward_getsockname, forward_getpeername, forward_shutdown with shadow socket support via resolve_lkl_socket - Shadow socket dup/dup2/dup3/fcntl(F_DUPFD) tracked via shadow_sp field with ADDFD injection and fd_table propagation - forward_close ref-counts shared lkl_fd before teardown - close_cloexec_entry ref-counts via lkl_fd_has_other_ref - forward_execve calls kbox_fd_table_close_cloexec for cleanup - fcntl(F_SETFD) syncs entry->cloexec for shadow sockets - host_fd close guarded by shadow_sp >= 0 (tracee-namespace FD) Change-Id: I48b05f90fac55f843330896f6b6bb84851ff1a65
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bridge LKL's virtio-net device with minislirp to provide outbound networking without privileges. Guest processes can resolve DNS and make HTTP connections through the host network via --net.
Core design:
Socket dispatch additions:
Change-Id: I48b05f90fac55f843330896f6b6bb84851ff1a65
Summary by cubic
Adds SLIRP user‑mode networking via
minislirpto enable outbound TCP/UDP (DNS, HTTP) from guests without privileges. Enable it with--net, which bridges LKL virtio‑net to a host SLIRP instance.New Features
net-dns-test, HTTP via wget.minislirpusing length‑prefixed pipes and an RX SPSC queue to avoid spin; event loop pumps frames.ADDFD; dispatcher forwards bind/connect/sendto/recv* and socket options; dup/close tracked.sendmsgstays allow‑listed for pre‑exec FD transfer—use sendto for addressed datagrams;recvmsgis intercepted to return source addresses.--netis used.Dependencies
externals/minislirpsubmodule withscripts/fetch-minislirp.sh;Makefileauto‑fetches whenKBOX_HAS_SLIRP=1(and.gitignoreupdated).kbox_notify_addfd_atfor dup2/dup3 injection.Written for commit ba08934. Summary will update on new commits.