Fused mirror scan, packed pair kernel, and lane pipelining for the CPU linear scan - #2348
Conversation
5e2b019 to
8ef678f
Compare
| run: | | ||
| curl -fsSL https://get.docker.com | sh | ||
| sudo usermod -aG docker $USER | ||
| sudo apt-get install acl | ||
| sudo setfacl --modify user:$USER:rw /var/run/docker.sock | ||
|
|
There was a problem hiding this comment.
Semgrep identified an issue in your code:
The release workflow executes the live response from get.docker.com with sh, allowing a compromised download to control the CI runner, Docker daemon, build context, and published image.
More details about this
The Install Docker step downloads https://get.docker.com and immediately executes its response with sh:
curl -fsSL https://get.docker.com | shBecause the downloaded bytes are executed before they are inspected, anyone who can compromise get.docker.com, its delivery path, or the content returned to this runner can run arbitrary commands as the GitHub Actions runner. This job runs on an arm64 Ubuntu runner for releases, has packages: write, contents: read, attestations: write, and id-token: write permissions, and later uses GITHUB_TOKEN to publish an image, making the runner a valuable target.
A plausible attack is:
- An attacker compromises the Docker install endpoint or causes the runner to receive a modified response for
get.docker.com. - The modified response is consumed by
shin theInstall Dockerstep, so it can read the checked-out repository, alter the Docker build context, and inspect available runner credentials or environment data. - The script can replace files used by the later
Build and pushstep, causing a malicious image to be published to${REGISTRY}/${IMAGE_NAME}. It can also tamper with the workflow workspace or wait for the laterdocker/login-actionstep to expose the registry token, then use that credential to push unauthorized packages. - The subsequent
sudo usermodandsetfaclcommands give the current user access to/var/run/docker.sock; code executed by the downloaded script can therefore control the Docker daemon and start privileged containers on the runner.
To resolve this comment:
✨ Commit fix suggestion
- Replace the
Install Dockerrun:step with the pinned Docker setup action:- name: Install Docker uses: docker/setup-docker-action@e43656e248c0bd0647d3f5c195d116aacf6fcaf4 with: version: v29.7.2
- Remove the
curl,usermod,apt-get, andsetfaclcommands. The setup action installs Docker without piping remote content directly into a shell and configures it for subsequent workflow steps.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by gha-curl-pipe-shell.
You can view more details about this finding in the Semgrep AppSec Platform.
- Port the GPU linear-scan bench to the resident-layout pool constructor; it still called the pre-layout LocalIrisWorkerPool::new_local and did not compile under --all-features --all-targets. - Restore the empty-request early return in search() and its test; the identity-update no-match search runs every batch and does not need a session per eye (the linear-scan no-match shortcut in per_session is back through the merge). - Lookahead dot tasks abort when their handle is dropped, so a lane error or a sibling lane failing try_join! cancels the in-flight chunk instead of leaving it running on the dot workers; the distance-mode and center rotation checks are made before spawning rather than inside the task. - Port the restored db-backed cold-eye test to the layout-aware pool API. Restored through the merge of the updated base: LinearScan-only gating of the full_face_mirror_* uniqueness result fields (HNSW keeps serializing null) and the dead linear-scan branch removal in per_session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAbAnQyfsSrWH6UGcRM1FF
The fused dot pass dispatches both orientations' queries through the normal-orientation store. That resolves the mirror query only because every session of an eye shares that eye's worker pool, into which HawkRequest::cache_into caches normal and mirror queries alike. Check the invariant per lane instead of relying on it silently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAbAnQyfsSrWH6UGcRM1FF
The HNSW-style windowed dot products read resident records through ResidentIris::to_arc, which rebuilds a u16 iris per target when the resident layout is the mixed-plane scan layout. The exact scan streams planes directly and never takes that path, so production pools now refuse it instead of running a silent per-target reconstruction if a future caller reaches it. The cross-kernel parity tests that compare both kernels on the same mixed-plane data opt in explicitly. Test actors pick the resident layout by search mode exactly like the server does (HNSW keeps u16, the exact scan uses the CPU's scan layout), through a shared HawkActor::resident_layout_for. Also take the cached live VectorId list from the registry in the paired cascade, matching the single-orientation path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XAbAnQyfsSrWH6UGcRM1FF
| /// `id` of the mask share (party_id + 1), preserved for reconstruction. | ||
| mask_id: usize, |
There was a problem hiding this comment.
This is never out of sync or am I missing something? why are we storing this twice?
| /// A spawned task handle that aborts the task when dropped, so a lookahead | ||
| /// chunk cannot keep running detached after the lane that requested it has | ||
| /// failed or been cancelled. | ||
| pub struct AbortOnDropHandle<T>(tokio::task::JoinHandle<T>); |
There was a problem hiding this comment.
This is in tokio-util: https://docs.rs/tokio-util/latest/tokio_util/task/struct.AbortOnDropHandle.html
| } | ||
| } | ||
|
|
||
| #[cfg(target_arch = "aarch64")] |
There was a problem hiding this comment.
These will not run in the CI pipeline I guess?
| /// independent dot products. | ||
| #[cfg(target_arch = "aarch64")] | ||
| #[inline(always)] | ||
| fn dot_product_1x4_u16(query: &[u16], targets: [&[u16]; 4]) -> [u16; 4] { |
There was a problem hiding this comment.
These could all be one const-generic function, no? I would hope the compiler can unroll this and produce equivalent code.
| let mut store = iris_store.data.write().await; | ||
| for (vector_id, iris) in resolved { | ||
| store.insert(vector_id, iris); | ||
| store.insert(vector_id, ResidentIris::from_arc(iris, layout)); |
There was a problem hiding this comment.
The from_arc does the conversion under the write lock here. I guess this is not really that much of an issue though
| row: usize, | ||
| rotation: usize, | ||
| ) -> *const u8 { | ||
| let amount = PrerotatedQueryRowMajorView::<ROTATIONS>::ROTATION_AMOUNTS[rotation]; |
There was a problem hiding this comment.
This could use a const assert that this is a multiple of 4.
| } | ||
| target_idx += 4; | ||
| } else { | ||
| // Missing vectors are uncommon in a full scan. Preserve their |
There was a problem hiding this comment.
I guess this is true, but maybe this is worth a metric emmission since this probably tanks performance if it happens often.
Summary
Adds the compute, memory-layout, networking, and scheduling optimizations for the exact CPU linear scan. It is stacked on the correctness baseline (#2347) and cold-eye cache/prefetch layer (#2351); it does not change thresholds, rotations, MPC result semantics, or the public server contract.
Actual server benchmark
Measured with the real
iris-mpc-linear-scanservice and production client across 3 × AWSr8g.24xlarge: TLS MPC networking, S3/SNS/SQS ingestion through Moto, persistence, normal + mirror scans, batch size 1, 31 rotations, and a 1,048,576-record seeded database.tokio=14)“Logical comparisons” counts normal and mirror work once rather than once per MPC party. In the 4.983 M/s run, median harness end-to-end throughput was 1.492 M comparisons/s because Moto SNS/SQS fan-out added about 985 ms; synchronized scan time is the production compute/network measurement. At 6.33 M/s, an 18M-record normal + mirror scan extrapolates to about 5.7 seconds, excluding ingress and persistence.
How it works
Capacity estimate: the resident mixed-plane eye costs about 38.55 kB per record, including container, allocator, registry, and 20% store-reserve overhead. The 501-record rolling LUC window, 4,096-entry TinyLFU, and one 4,096-record prefetch chunk add about 0.32 GiB. On an
r8g.24xlargewith the configured 700 GiB pod limit, the recommended operational maximum is 18–18.5M records (about 646.5–664.5 GiB); the roughly 19.49M mathematical ceiling leaves no safe runtime margin.Deployment
ghcr.io/worldcoin/iris-mpc-linear-scanusing.github/workflows/build-and-push-linear-scan-server.yaml. Pin production to an immutable image digest.r8g.24xlargeper party and use the supplied stage/prod common values plus party overlays. They request 92 CPUs without a CPU limit and select/tolerate the dedicated linear-scan nodes.deploy/aws/r8g-24xlarge-linear-scan-user-data.mimethrough the node class, launch template, or baked AMI. It keeps ENA interrupts on CPUs 0–10 and reserves CPUs 11–95 for dot-product workers; verify IRQ placement under load.hostNetwork: true, allow direct cross-party TCP ports 4000–4002, and keep 16 physical connections so traffic is not capped by a single AWS 5-Gbit/s flow.SMPC__MAX_BATCH_SIZE=1, the sameSMPC__FULL_SCAN_SIDEon every party,SMPC__FULL_SCAN_SIDE_SWITCHING_ENABLED=false, all three/readyendpoints, a real request, and the scan/cache metrics.Recreate. Never run mixed images or both queue consumers concurrently. Alternate the resident eye only by changingLeft/Righton all parties and performing another coordinated restart.Review guide
iris-mpc-cpu/src/execution/hawk_main/search.rs.worker_pool_initializer.rsandiris_worker.rs.iris-mpc-cpu/src/protocol/ops.rsplus fused MPC primitives inampc-common.Stack