Skip to content

learn: prototype sandlock learn subcommand#113

Open
ghazariann wants to merge 18 commits into
multikernel:mainfrom
ghazariann:feat/learn
Open

learn: prototype sandlock learn subcommand#113
ghazariann wants to merge 18 commits into
multikernel:mainfrom
ghazariann:feat/learn

Conversation

@ghazariann

Copy link
Copy Markdown
Contributor

Prototype implementation of sandlock learn (#72).

Summary

Adds sandlock learn -o profile.toml -- <cmd> which runs a workload under observation
and writes a sandlock profile TOML directly usable by sandlock run -p profile.toml.

Domain Implementation
Filesystem reads on_file_access audit hook in supervisor on openat / open / execve / execveat
Filesystem writes same hook; classified by open flags (O_WRONLY / O_RDWR / O_CREAT)
Network egress (TCP / UDP) on_net_connect audit hook in supervisor on connect / sendto / sendmsg
HTTP method + host + path not yet
Syscalls not yet
Resource peaks not yet

Implementation

Runs the workload under fully-permissive Landlock and intercepts syscalls via two
audit hooks added to the sandlock-core supervisor, called before dispatch on every
notification:

  • on_file_access(path, flags)openat / open / execve / execveat
  • on_net_connect(ip, port)connect / sendto / sendmsg

Results are collected and serialized to a ProfileInput TOML.

Discussion points

1. Capturing the executed binary

For sandlock learn -- cat /etc/hostname, /usr/bin/cat must appear in the profile so sandlock run can allow it via Landlock. The binary is loaded through execve, not openat, so the on_file_access hook alone is not enough.

Adding execve to the hook condition in handle_notification was not sufficient on its own. The seccomp BPF filter decides which syscalls generate a SECCOMP_RET_USER_NOTIF, and for a basic sandbox (fs_read, fs_write) execve was not in that list. It only entered notif_syscalls for heavier features (COW, chroot). The notification never reached the supervisor.

The fix: a new audit_file_access feature flag in SandboxFeatures that is true when on_file_access is set. In notif_syscalls_resolved this adds execve/execveat to the BPF notif list. resolve_path_for_notif already handled execve, so no other supervisor logic changed.

Is this the right place to wire this? Should execve have its own separate hook (on_execve) instead of being folded into on_file_access?

2. Resolving the dynamic linker

After execve the kernel maps the dynamic linker (e.g. /lib64/ld-linux-x86-64.so.2) in kernel space before transferring control to userspace, no syscall fires, so it never appears in the on_file_access trace. Without it in the profile, sandlock run fails: Landlock blocks the read of the linker and the process cannot start at all.

The current workaround parses the ELF PT_INTERP segment of the binary (ELF64 only) to recover the interpreter path. This is ad-hoc and not portable (assumes ELF64, specific header offsets, and manual endianness handling).

One idea: the linker appears in /proc/<pid>/maps after execve completes. The supervisor already reads /proc/<pid>/maps for vDSO patching (maybe_patch_vdso), so the pattern exists. But im not sure with the timing cause the execve notification fires before the kernel completes exec.

I'm not that experienced in kernel development and would love guidance on the right approach here.

3. Achieving permissive Landlock during observation

The spec calls for permissive Landlock + seccomp-notify during observation. For reads this is straightforward: .fs_read("/"). For writes, .fs_write("/") is not an option: the observation run must be non-destructive, leaving no trace on the real filesystem.

The current approach pairs .fs_read("/") with .workdir(tempdir) (COW overlay): writes are granted everywhere but redirected to a temporary overlay, so the real filesystem is never touched. The on_file_access hook fires before the COW redirect, so it always sees the original requested path, which is what ends up in the profile.

Is COW the right way to achieve a fully permissive observation environment? Is there a lighter approach?

What still needs to be done

  • Path collapsing
  • Merging and iteration
  • HTTP method + host + path (http.allow)
  • Syscall counting (--learn-syscalls)
  • Resource peaks via /proc sampling

Tests

  • test_learn_captures_fs_read — runs cat /etc/hostname, checks /etc/hostname appears under read
  • test_learn_then_run — full round-trip: learn generates profile from cat /etc/hostname, sandlock run uses it
  • test_learn_captures_fs_write — writes to a pre-existing NamedTempFile (file exists before learn runs), checks path appears under write
  • test_learn_new_file_collapses_to_parent — writes to a file that does not exist; checks the profile records the parent directory, and the real file is never created (COW isolation)
  • test_learn_then_run_write — round-trip for writes: learn captures a write, run actually creates the file
  • test_learn_captures_net_connect — binds a real TcpListener, runs a Python connect, checks the address appears under [network] allow
  • test_learn_then_run_network — round-trip for network: single listener accepts two connections, one from learn and one from run

Test plan

  • cargo test -p sandlock-cli test_learn
  • sandlock learn -o /tmp/profile.toml -- <cmd>
  • sandlock run -p /tmp/profile.toml -- <cmd>

@congwang-mk

Copy link
Copy Markdown
Contributor

Hi @ghazariann

Thanks for your PR! It looks nice from my quick glance.

Regarding your questions:

  1. Using a separate on_execve() hook looks better.
  2. Drop the hand-written ELF parser if possible, you can exactly follow maybe_patch_vdso().
  3. COW is the right call.

@congwang-mk

Copy link
Copy Markdown
Contributor

Hi @ghazariann

Thanks for working on it.

One design question: why not leveraging the existing SysscallEvent provided by policy_fn ? If it misses anything you need here, we could extend it?

@ghazariann

Copy link
Copy Markdown
Contributor Author

Hi @ghazariann

Thanks for working on it.

One design question: why not leveraging the existing SysscallEvent provided by policy_fn ? If it misses anything you need here, we could extend it?

Hi @congwang-mk, that makes more sense. I have extended SyscallEvent with path and flags so the callback can see which file was opened and with what flags.

My only concern is TOCTOU. Exposing the path for openat can create race issues (issue #27), as mentioned in the comment. Any ideas on how to get the path to learn safely?

Also added resource peak sampling ([limits].memory, [limits].processes, [limits].open_files are now populated from observed peaks).

@congwang-mk

Copy link
Copy Markdown
Contributor

@ghazariann Thanks for the update!

Your concern is valid. The TOCTOU is very hard to resolve without help from kernel. This is why I submitted a linux kernel patchset to fix this: https://lore.kernel.org/all/20260704231831.354543-1-xiyou.wangcong@gmail.com/ On the bright side, this PR does not make it worse.

@congwang-mk congwang-mk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR. Some comments below:

}
"connect" | "sendto" => {
if let (Some(ip), Some(port)) = (event.host, event.port) {
self.connects.lock().unwrap().insert(format!("tcp://{ip}:{port}"));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need to distinguish UDP and TCP here?

pub denied: bool,
/// Resolved absolute path for file syscalls (openat, execve/execveat).
/// Read from the kernel's fd table via `/proc/<pid>/fd/<fd>` after the
/// supervisor's on-behalf open — not from child user memory — so it is

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we read from child's memory via read_path_for_event(). So this comment needs to be updated?

// openat(dirfd, pathname, flags, mode): resolved path + flags.
// Path is read via /proc/<pid>/fd after the on-behalf open — TOCTOU-safe.
// openat2 uses struct open_how* for flags so flags is left None there.
if nr == libc::SYS_openat || Some(nr) == arch::sys_open() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is openat2() missing here?


sampler.abort();

eprintln!("sandlock learn: done (exit={:?})", result.code());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the target program exits abnormally, should we still continue writing out its profile?

@congwang-mk

Copy link
Copy Markdown
Contributor

Two more comments:

  1. It seems only open-family is learned, other filesystem syscalls (like mkdir, unlink) are missing
  2. --timeout might be useful for sandlock learn too

@dzerik

dzerik commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Some thoughts on sandlock learn

We got interested in this PR and spent a while reading through it alongside the core — really like where learn is heading. Sharing where we landed on the things you flagged, plus a couple we ran into, in case any of it is useful. None of it is blocking from our side — take or leave whatever's helpful.

(A) Linker timing

We think your timing instinct is right, and the code already handles it well. At the execve notification the kernel hasn't committed the new mm yet — emit_policy_event (notif.rs:1813) runs before the Continue/send_response at :1883, and USER_NOTIF fires at syscall entry, before load_elf_binary maps the interpreter — so a synchronous /proc/<pid>/maps read there would show the old image. Your pending_maps deferred read on the pid's next notification sidesteps that nicely (it mirrors what maybe_patch_vdso does), and it has a bonus over the ELF PT_INTERP parser: it captures whatever loader the kernel actually mapped, right arch/libc and all. So our vote would be to keep the deferred read and drop the PT_INTERP path.

A couple of small things we noticed: there's an orphaned comment left from the pivot around notif.rs:1230-1236; and one edge worth a thought — notif.pid is the triggering TID, so if a non-leader thread does the execve, de_thread reassigns the pid and the next notification may carry a different one than pending_maps stored. Keying/falling back on the TGID (or reading maps the first time any unseen pid appears) would close it. /proc/<pid>/exe is also a nice race-free source for the exec path if you want it.

(B) TOCTOU

For what it's worth, we don't think there's a security TOCTOU in learn at all — the callback always returns Verdict::Allow, the sandbox is permissive, the path only feeds the TOML, and the profile gets re-validated in-kernel by static Landlock at run (where paths resolve race-free). So a mis-recorded path can only affect the output, never enforcement.

The one thing we'd flag: the doc comment on SyscallEvent.path says the path comes from the fd table via /proc/<pid>/fd/<fd> and is "TOCTOU-safe" — but for openat it's actually read from child memory (read_child_mem, notif.rs:1325) and lexically normalized. Might be worth correcting that (and its twin at notif.rs:1593-1594, and the execve "resolved via procfs" note at notif.rs:1574) so nobody downstream relies on a safety property that isn't there — the real reason it's fine here is that learn makes no decision, not the source of the string. The residual symlink-swap race is a profile-quality thing (record a wrong path → re-run), so a doc note feels like enough; probably not worth reaching for the execve-freeze machinery.

(C) Coverage

This one surprised us a bit in a good way: because learn turns on COW, the FS-mutating syscalls are already in the notif set and already reach your callback — they just get dropped in the layers above learn.rs. So it looks like recording a mutation needs four things lined up: adding the mutators to POLICY_EVENT_SYSCALLS (seccomp_plan.rs) so observation doesn't quietly depend on COW; populating the path (today emit_policy_event only fills it for open/execve at notif.rs:1596 — a branch plus a few resolve_path_for_notif arms like mkdirat/unlinkat/rmdir/mknodat/truncate, though linkat/renameat2/rename arms already exist); adding syscall_name arms (several currently return "unknown" and get dropped); and the matching on_event arms in learn.rs.

The subtlety that cost us the most reasoning is the Landlock rights model: MAKE_*/REMOVE_*/REFER are directory-only rights, so it seems like every create/remove/rename/mkdir/symlink/mknod wants to be recorded as fs_write on the parent directory rather than the target — otherwise run rejects them, and the current collapse at learn.rs:214-223 (which keeps the file path when it exists) would mis-record unlink/rmdir/rename. A few arg-layout gotchas we hit: symlinkat's created path is args[2] (and args[0] is the target string, not a path to grant); unlinkat needs args[2] & AT_REMOVEDIR to tell unlink from rmdir; rename/link are two-path; mknod needs per-arch gating. Our rough take on a first cut: the enforced-by-Landlock ones (mkdir(at), unlink(at), rmdir, rename(at2), link(at), symlink(at), mknod(at), truncate, AF_UNIX bind) are the ones that actually make run fail if missing, whereas stat/access/readlink/getdents/chmod/chown/utimensat/mount/chdir have no Landlock right at all, so recording them would mostly just add noise.

(D) Architecture

The policy_fn/SyscallEvent seam feels right to us and it self-wires (features.policy_fn from policy_fn.is_some()), so we don't think you need a separate on_execve — execve gating comes along automatically. A few things we noticed: openat2 opens seem to be dropped silently (path+flags stay None, name "unknown") — decode_open_args at notif.rs:494 already decodes it, so an arm there would close it (narrow in practice, since glibc/musl/Go use plain openat). --timeout isn't wired yet — wrapping the first wait in tokio::time::timeout and falling back to sandbox.kill() + an unbounded re-wait would do it. On resource peaks, one thing to watch: sandlock doesn't use cgroups, and handle_memory charges virtual anonymous allocation rather than RSS, so a VmHWM-derived limits.memory can mismatch what run enforces — running learn with a finite max_memory and reading the peak mem_used round-trips exactly, otherwise leaving it unset might be safer than a wrong value. Sampling the process subtree (and taking one sample right after start()) would also catch short-lived and multi-process workloads that the current 100ms-first loop misses.

(E) Profile quality

Mostly a single post-processing pass over the collected sets before building FilesystemSection, we think. A read grant on a directory is recursive in Landlock (add_path_rule installs PATH_BENEATH), so collapsing many /usr/lib/... leaves into read="/usr/lib" stays faithful — probably worth being aggressive for reads over immutable trees and conservative (parent-only) for writes, then dropping any leaf a retained ancestor already covers. One thing to keep in mind: in the non-chroot path a missing rule path aborts confine entirely (landlock.rs:487, no existence skip), so re-checking existence after collapsing matters. We also think there's a real bug at learn.rs:220: it collapses a new write to its immediate parent only, so an absolute nested create whose sub-dirs don't exist yet (COW intercepted the mkdirs) drops the write and run can't recreate it — walking p.ancestors() to the nearest existing dir would fix it (a test with an absolute nested path would catch it). Smaller notes: a deny-prefix for /proc, /dev/pts, /dev/tty would keep pid-specific junk out; the 256-byte path cap in read_child_mem silently drops long paths; and paths are lexically normalized rather than canonicalized. A --merge mode across runs (union the sets, element-wise max the limits) seems like a natural follow-up rather than v1.

How we might sequence it

If it's helpful, here's roughly the order we'd tackle it in: the doc-comment fixes first (cheap, and they clear up the safety story); then the nested-write ancestor fix; the junk deny-list plus prefix-subsumption dedup; and reliable linker/library capture (unioning file-backed r-xp maps across the tree in the sampler, keeping pending_maps for descendants). Those four alone would give a prototype that round-trips reliably. Then the FS-mutation coverage and the --timeout/resource fixes fill out completeness, with openat2, canonicalization, and --merge as polish afterward.

Really nice work getting this far — happy to pair on any of these or dig deeper wherever it'd help.

@dzerik

dzerik commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Quick follow-up on the path-collapsing side, since it ties straight into the discussion in #72 — and we think it's one of the more important application aspects here, the thing that separates a genuinely minimal profile from "-r /etc with extra steps."

In #72 the collapser safety came up and the design landed on three guards: no collapse by default, a hard sensitive-prefix deny-list that's never aggregated regardless of how many files were touched (/etc, /proc, /sys, /dev, /root, ~/.ssh, ~/.aws, ~/.kube, ~/.gnupg), and a non-optional observed-vs-granted diff so the operator consciously sees what a collapse opens up.

Why we're flagging it here: the current prototype already does one directory collapse today, on the write side, without those guards. For a new file that COW intercepted, learn.rs:214-223 records p.parent() as fs_write. Since a Landlock write grant on a directory is recursive (and write_access() also bundles the MAKE_*/REMOVE_* rights), a workload that creates a single new file under a sensitive dir silently yields a recursive grant over the whole thing:

  • new file /etc/foofs_write = "/etc" (write+create+delete across all of /etc, incl. /etc/shadow, /etc/ssh/)
  • a new file directly at /foo → parent is /fs_write = "/", i.e. write over the entire filesystem

None of those siblings were observed — exactly the "minimum-so-it-runs ≠ minimum-permission" divergence from #72.

The write side is actually the trickier one, because unlike reads you can't just keep the leaf: Landlock has no "create only this one file" right — a new file inherently needs a write grant on an existing directory, and in the non-chroot path a non-existent rule path aborts confine outright (landlock.rs:487). So the collapse isn't optional here, which makes the #72 guards more load-bearing on writes, not less — the sensitive-prefix deny-list and the observed-vs-granted diff are what keep "create one file under /etc" from silently becoming "write all of /etc." Our suggestion would be to wire the #72 backstop into this write-collapse now (deny-list + diff, and flag or refuse a sensitive/too-shallow parent like /, /etc, $HOME), before the read-side collapse gets layered on top — otherwise the read-collapse lands on a write path that already crosses the line.

Not trying to expand scope — just that this is the one spot where "don't collapse blindly" is already live in the code, and it's the aspect #72 spent the most time on, so it felt worth connecting the two.

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.

3 participants