Skip to content

Commit

Permalink
Fix Go >=1.23.3 crash due to running hook in vfork (#2989)
Browse files Browse the repository at this point in the history
* asm glue fixed on x64

* Changelog

* integration test

* fmt

* Comment style
  • Loading branch information
Razz4780 authored Dec 22, 2024
1 parent 8deaa2c commit 30bcfcc
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog.d/2988.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed panic when Go >=1.23.3 verifies pidfd support on Linux.
13 changes: 12 additions & 1 deletion mirrord/layer/src/go/linux_x64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,10 @@ unsafe extern "C" fn c_abi_syscall_handler(
#[naked]
unsafe extern "C" fn go_syscall_new_detour() {
naked_asm!(
"cmp rax, 60", // SYS_EXIT
"je 4f",
"cmp rax, 231", // SYS_EXIT_GROUP
"je 4f",
// Save rdi in r10
"mov r10, rdi",
// Save r9 in r11
Expand Down Expand Up @@ -489,7 +493,14 @@ unsafe extern "C" fn go_syscall_new_detour() {
"mov rcx, 0x0",
"xorps xmm15, xmm15",
"mov r14, QWORD PTR FS:[0xfffffff8]",
"ret"
"ret",
// just execute syscall instruction
// This is for SYS_EXIT and SYS_EXIT_GROUP only - we know for sure that it's safe to just
// let it happen. Running our code is an unnecessary risk due to switching between
// stacks. See issue https://github.com/metalbear-co/mirrord/issues/2988.
"4:",
"mov rdx, rdi",
"syscall",
)
}

Expand Down
3 changes: 3 additions & 0 deletions mirrord/layer/tests/apps/issue2988/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module issue2988

go 1.23.4
13 changes: 13 additions & 0 deletions mirrord/layer/tests/apps/issue2988/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package main

import (
_ "net"
"os"
)

func main() {
_, err := os.FindProcess(os.Getpid())
if err != nil {
panic(err)
}
}
9 changes: 7 additions & 2 deletions mirrord/layer/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,8 @@ pub enum Application {
},
// For running applications with the executable and arguments determined at runtime.
DynamicApp(String, Vec<String>),
// Go app that only checks whether Linux pidfd syscalls are supported.
Go23Issue2988,
}

impl Application {
Expand Down Expand Up @@ -958,6 +960,7 @@ impl Application {
Application::RustIssue2204 => String::from("tests/apps/issue2204/target/issue2204"),
Application::Go23Open { .. } => String::from("tests/apps/open_go/23.go_test_app"),
Application::DynamicApp(exe, _) => exe.clone(),
Application::Go23Issue2988 => String::from("tests/apps/issue2988/23.go_test_app"),
}
}

Expand Down Expand Up @@ -1081,7 +1084,8 @@ impl Application {
| Application::CIssue2178
| Application::RustIssue2204
| Application::RustRebind0
| Application::RustIssue2438 => vec![],
| Application::RustIssue2438
| Application::Go23Issue2988 => vec![],
Application::RustOutgoingUdp => ["--udp", RUST_OUTGOING_LOCAL, RUST_OUTGOING_PEERS]
.into_iter()
.map(Into::into)
Expand Down Expand Up @@ -1175,7 +1179,8 @@ impl Application {
| Application::NodeIssue2807
| Application::RustRebind0
| Application::Go23Open { .. }
| Application::DynamicApp(..) => unimplemented!("shouldn't get here"),
| Application::DynamicApp(..)
| Application::Go23Issue2988 => unimplemented!("shouldn't get here"),
Application::PythonSelfConnect => 1337,
Application::RustIssue2058 => 1234,
}
Expand Down
23 changes: 23 additions & 0 deletions mirrord/layer/tests/issue2988.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#![feature(assert_matches)]
#![warn(clippy::indexing_slicing)]
use std::{path::Path, time::Duration};

use rstest::rstest;

mod common;

pub use common::*;

/// Verify that issue [#2988](https://github.com/metalbear-co/mirrord/issues/2988) is fixed.
#[rstest]
#[tokio::test]
#[timeout(Duration::from_secs(60))]
async fn test_issue2988(
#[values(Application::Go23Issue2988)] application: Application,
dylib_path: &Path,
) {
let (mut test_process, _intproxy) = application
.start_process_with_layer(dylib_path, vec![], None)
.await;
test_process.wait_assert_success().await;
}

0 comments on commit 30bcfcc

Please sign in to comment.