diff --git a/Makefile b/Makefile index 0f990a25..dab93f4f 100644 --- a/Makefile +++ b/Makefile @@ -116,8 +116,19 @@ PARROT_TOOL ?= $(TOOL_BIN)/parrot$(exe_suffix) PARROT_TOOL_CONTAINER_BINARY ?= $(TOOL_BIN)/parrot_c CONTAINER_PROBE_TOOL_CONTAINER_BINARY ?= $(TOOL_BIN)/container_probe_c TERMCHILD_TOOL ?= $(TOOL_BIN)/termchild$(exe_suffix) +SIGNAL_DISPOSITION_TOOL ?= $(TOOL_BIN)/signal-disposition$(exe_suffix) GO_LICENSES ?= $(TOOL_BIN)/go-licenses$(exe_suffix) PROTOC ?= $(TOOL_BIN)/protoc/bin/protoc$(exe_suffix) +GO_RUNTIME_OVERLAY_DIR ?= $(TOOL_BIN)/go-runtime-overlay +GO_RUNTIME_OVERLAY_FILE ?= $(GO_RUNTIME_OVERLAY_DIR)/overlay.json + +ifeq ($(build_os),darwin) + GO_RUNTIME_OVERLAY_ARG := -overlay="$(GO_RUNTIME_OVERLAY_FILE)" + GO_RUNTIME_OVERLAY_PREREQ := $(GO_RUNTIME_OVERLAY_FILE) +else + GO_RUNTIME_OVERLAY_ARG := + GO_RUNTIME_OVERLAY_PREREQ := +endif # Tool Versions PROTOC_VERSION ?= 33.5 @@ -167,6 +178,9 @@ help: ## Display this help. ##@ Code generation +.PHONY: generate-go-runtime-overlay +generate-go-runtime-overlay: $(GO_RUNTIME_OVERLAY_PREREQ) ## Generate the Darwin Go runtime overlay used by DCP builds + .PHONY: generate generate: generate-object-methods generate-openapi generate-goversioninfo generate-grpc ## Generate artifacts needed for DCP binary build: object copy methods, OpenAPI definitions, binary version info, and gRPC files. @@ -293,8 +307,8 @@ build-ci: generate-ci release ## Runs codegen, including license/notice files, t .PHONY: build-dcp build-dcp: $(DCP_BINARY) ## Builds DCP CLI binary -$(DCP_BINARY): $(GO_SOURCES) go.mod | ${OUTPUT_BIN} - $(GO_BIN) build -o $(DCP_BINARY) $(BUILD_ARGS) ./cmd/dcp +$(DCP_BINARY): $(GO_SOURCES) go.mod $(GO_RUNTIME_OVERLAY_PREREQ) | ${OUTPUT_BIN} + $(GO_BIN) build -o $(DCP_BINARY) $(GO_RUNTIME_OVERLAY_ARG) $(BUILD_ARGS) ./cmd/dcp .PHONY: build-dcptun-containerexe build-dcptun-containerexe: $(DCPTUN_CLIENT_BINARY) ## Builds DCP reverse network tunnel client binary for Linux (to be used in containers) @@ -344,6 +358,9 @@ TEST_PREREQS := generate-grpc .WAIT build-dcp build-dcptun-containerexe containe else TEST_PREREQS := generate-grpc build-dcp build-dcptun-containerexe container-probe-tool-containerexe delay-tool lfwriter-tool parrot-tool parrot-tool-containerexe termchild-tool endif +ifeq ($(build_os),darwin) +TEST_PREREQS := $(TEST_PREREQS) signal-disposition-tool +endif .PHONY: test-prereqs test-prereqs: BUILD_ARGS := $(BUILD_ARGS) -gcflags="all=-N -l" -ldflags "$(version_values)" @@ -361,13 +378,13 @@ TEST_OPTS := $(COMMON_TEST_OPTS) -race endif .PHONY: test -test: test-prereqs ## Run all tests in the repository - $(GO_BIN) test ./... $(TEST_OPTS) -parallel 32 +test: test-prereqs $(GO_RUNTIME_OVERLAY_PREREQ) ## Run all tests in the repository + $(GO_BIN) test ./... $(GO_RUNTIME_OVERLAY_ARG) $(TEST_OPTS) -parallel 32 # NOTE: Keep scripts/test-ci.ps1 in sync with test-ci (see comment above TEST_PREREQS). .PHONY: test-ci -test-ci: test-ci-prereqs ## Runs tests in a way appropriate for CI pipeline, with linting etc. - $(GO_BIN) test ./... $(TEST_OPTS) +test-ci: test-ci-prereqs $(GO_RUNTIME_OVERLAY_PREREQ) ## Runs tests in a way appropriate for CI pipeline, with linting etc. + $(GO_BIN) test ./... $(GO_RUNTIME_OVERLAY_ARG) $(TEST_OPTS) ## Development and test support targets @@ -381,6 +398,15 @@ ${OUTPUT_BIN}/ext/bin/: | ${OUTPUT_BIN} $(TOOL_BIN): $(mkdir) $(TOOL_BIN) +ifeq ($(build_os),darwin) +.PHONY: force-go-runtime-overlay +force-go-runtime-overlay: +# Apply the upstream fix for golang/go#81009 to the selected toolchain without +# replacing unrelated standard-library source. +$(GO_RUNTIME_OVERLAY_FILE): force-go-runtime-overlay $(wildcard ./internal/tools/goruntimeoverlay/*) | $(TOOL_BIN) + $(CLEAR_GOARGS) $(GO_BIN) run ./internal/tools/goruntimeoverlay --output-dir "$(GO_RUNTIME_OVERLAY_DIR)" +endif + $(DCP_DIR): $(mkdir) $(DCP_DIR) @@ -405,26 +431,35 @@ endif # delay-tool is used for process package testing .PHONY: delay-tool delay-tool: $(DELAY_TOOL) -$(DELAY_TOOL): $(wildcard ./test/delay/*.go) | $(TOOL_BIN) - $(GO_BIN) build -o $(DELAY_TOOL) github.com/microsoft/dcp/test/delay +$(DELAY_TOOL): $(wildcard ./test/delay/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) + $(GO_BIN) build -o $(DELAY_TOOL) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/delay # termchild-tool is used for internal/termpty pseudo-terminal tests .PHONY: termchild-tool termchild-tool: $(TERMCHILD_TOOL) -$(TERMCHILD_TOOL): $(wildcard ./test/termchild/*.go) | $(TOOL_BIN) - $(GO_BIN) build -o $(TERMCHILD_TOOL) github.com/microsoft/dcp/test/termchild +$(TERMCHILD_TOOL): $(wildcard ./test/termchild/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) + $(GO_BIN) build -o $(TERMCHILD_TOOL) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/termchild + +# signal-disposition captures the signal state inherited by an exec'd child before +# its Go runtime initializes. +ifeq ($(build_os),darwin) +.PHONY: signal-disposition-tool +signal-disposition-tool: $(SIGNAL_DISPOSITION_TOOL) +$(SIGNAL_DISPOSITION_TOOL): $(wildcard ./test/signaldisposition/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) + CGO_ENABLED=1 $(GO_BIN) build -o $(SIGNAL_DISPOSITION_TOOL) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/signaldisposition +endif # lfwriter tool is used for testing lockfile package .PHONY: lfwriter-tool lfwriter-tool: $(LFWRITER_TOOL) -$(LFWRITER_TOOL): $(wildcard ./test/lfwriter/*.go) | $(TOOL_BIN) - $(GO_BIN) build -o $(LFWRITER_TOOL) github.com/microsoft/dcp/test/lfwriter +$(LFWRITER_TOOL): $(wildcard ./test/lfwriter/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) + $(GO_BIN) build -o $(LFWRITER_TOOL) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/lfwriter # parrot tool is used for testing network connectivity .PHONY: parrot-tool parrot-tool: $(PARROT_TOOL) -$(PARROT_TOOL): $(wildcard ./test/parrot/*.go) | $(TOOL_BIN) - $(GO_BIN) build -o $(PARROT_TOOL) github.com/microsoft/dcp/test/parrot +$(PARROT_TOOL): $(wildcard ./test/parrot/*.go) $(GO_RUNTIME_OVERLAY_PREREQ) | $(TOOL_BIN) + $(GO_BIN) build -o $(PARROT_TOOL) $(GO_RUNTIME_OVERLAY_ARG) github.com/microsoft/dcp/test/parrot # Builds a static parrot binary suitable for the scratch-based test container image. .PHONY: parrot-tool-containerexe diff --git a/internal/dcp/commands/root.go b/internal/dcp/commands/root.go index de9944f3..8c4bd106 100644 --- a/internal/dcp/commands/root.go +++ b/internal/dcp/commands/root.go @@ -93,12 +93,6 @@ func NewRootCmd(log *logger.Logger) (*cobra.Command, error) { rootCmd.AddCommand(cmd) } - if cmd, err = dcpproc_cmds.NewForkProcessExecCommand(log.Logger); err != nil { - return nil, fmt.Errorf("could not set up '%s' command: %w", dcpproc_cmds.ForkProcessExecCmdName, err) - } else { - rootCmd.AddCommand(cmd) - } - // Add dcptun sub-commands rootCmd.AddCommand(dcptun_cmds.NewRunServerCommand(log.Logger)) diff --git a/internal/dcpproc/commands/fork_process.go b/internal/dcpproc/commands/fork_process.go index 1e06b45e..4d0bb725 100644 --- a/internal/dcpproc/commands/fork_process.go +++ b/internal/dcpproc/commands/fork_process.go @@ -8,12 +8,8 @@ package commands import ( "context" "fmt" - "io" "os" "os/exec" - "strconv" - "strings" - "syscall" "github.com/go-logr/logr" "github.com/spf13/cobra" @@ -61,14 +57,6 @@ func forkProcess(log logr.Logger) func(cmd *cobra.Command, args []string) error logger.WithSessionId(childCmd) process.ForkFromParent(childCmd) - execShim, shimErr := useExecShim(childCmd) - if shimErr != nil { - return shimErr - } - if execShim != nil { - defer execShim.close() - } - monitorEnabled := cmd.Flags().Changed("monitor") var monitorCtx context.Context var monitorCtxCancel context.CancelFunc @@ -83,7 +71,7 @@ func forkProcess(log logr.Logger) func(cmd *cobra.Command, args []string) error } } - pid, childExitInfoCh, disposeChildExecutor, startErr := startForkedProcess(cmd, childCmd, execShim, monitorEnabled, log) + pid, childExitInfoCh, disposeChildExecutor, startErr := startForkedProcess(cmd, childCmd, monitorEnabled, log) if startErr != nil { return startErr } @@ -127,7 +115,6 @@ func forkProcess(log logr.Logger) func(cmd *cobra.Command, args []string) error func startForkedProcess( cmd *cobra.Command, childCmd *exec.Cmd, - execShim *execShimHandshake, observeExit bool, log logr.Logger, ) (process.Pid_t, <-chan process.ProcessExitInfo, func(), error) { @@ -154,18 +141,6 @@ func startForkedProcess( return process.UnknownPID, nil, nil, fmt.Errorf("could not start forked process: %w", startErr) } - // Starting the shim only means dcp itself started. The PID must not be reported before the - // requested program is known to be running, so that a program which cannot be executed is - // still reported as a start failure. - if execShim != nil { - if execErr := execShim.wait(); execErr != nil { - // The logger already carries the command and arguments. - log.Error(execErr, "Failed to execute forked process") - executor.Dispose() - return process.UnknownPID, nil, nil, fmt.Errorf("could not start forked process: %w", execErr) - } - } - pid := handle.Pid if _, writeErr := fmt.Fprintln(cmd.OutOrStdout(), pid); writeErr != nil { log.Error(writeErr, "Failed to write forked process PID", "PID", pid) @@ -188,115 +163,3 @@ func trimForkProcessArgSeparator(args []string) []string { return args } - -// Redirects the child through the 'fork-process-exec' command on platforms where the child would -// otherwise inherit an invalid SIGUSR1 disposition from the Go runtime. The shim cleans that -// disposition and then execs the original program, which keeps the process ID, session, standard -// streams, and exit code that the caller of 'fork-process' expects. -// -// The reset cannot be done here: the Go runtime restores its own signal dispositions in the -// forked child before it reaches execve, so it has to happen in the process that calls exec. -// -// Returns the handshake that reports whether the shim reached the requested program, or nil when -// the child is started directly. The caller owns the returned handshake and must close it. -func useExecShim(childCmd *exec.Cmd) (*execShimHandshake, error) { - if !process.NeedsExecSignalDispositionWorkaround() { - return nil, nil - } - - if childCmd.Err != nil { - // The program could not be located. Leave the command untouched so that starting it - // reports that original failure rather than one from the shim. - return nil, nil - } - - callerSIGUSR1Ignored, dispositionErr := process.InheritedSIGUSR1Ignored() - if dispositionErr != nil { - return nil, fmt.Errorf("could not determine the inherited SIGUSR1 disposition: %w", dispositionErr) - } - - return useExecShimWithDisposition(childCmd, callerSIGUSR1Ignored) -} - -func useExecShimWithDisposition( - childCmd *exec.Cmd, - callerSIGUSR1Ignored bool, -) (*execShimHandshake, error) { - dcpPath, dcpPathErr := os.Executable() - if dcpPathErr != nil { - return nil, fmt.Errorf("could not determine the path of the current executable: %w", dcpPathErr) - } - - statusR, statusW, pipeErr := os.Pipe() - if pipeErr != nil { - return nil, fmt.Errorf("could not create the exec status pipe: %w", pipeErr) - } - - shimArgs := []string{ - dcpPath, - ForkProcessExecCmdName, - "--" + execPathFlagName, childCmd.Path, - "--" + callerSIGUSR1IgnoredFlagName + "=" + strconv.FormatBool(callerSIGUSR1Ignored), - } - shimArgs = append(shimArgs, "--") - childCmd.Args = append(shimArgs, childCmd.Args...) - childCmd.Path = dcpPath - - // The shim reports the outcome of the exec on this descriptor. It is the only extra file, so - // the shim sees it as execStatusFd. - childCmd.ExtraFiles = append(childCmd.ExtraFiles, statusW) - - return &execShimHandshake{statusR: statusR, statusW: statusW}, nil -} - -// execShimHandshake reports whether the shim managed to exec the requested program. Starting the -// shim only proves that dcp itself could be started, so without this the caller would be told -// that a program which never ran had started successfully. -// -// The shim inherits the write end. A successful execve closes it and the read end reports EOF, -// while a failure sends the errno before the shim exits. -type execShimHandshake struct { - statusR *os.File - statusW *os.File -} - -// wait blocks until the shim either replaces itself with the requested program or reports why it -// could not. It returns the failure that a direct start would have reported. -func (h *execShimHandshake) wait() error { - // The write end is now owned by the shim. The parent's copy has to go, because the read below - // only reports EOF once every writer is closed. - h.closeWriteEnd() - - status, readErr := io.ReadAll(h.statusR) - if readErr != nil { - return fmt.Errorf("could not read the exec status: %w", readErr) - } - - if len(status) == 0 { - // EOF with nothing written: the descriptor was closed by a successful execve. - return nil - } - - errnoValue, parseErr := strconv.Atoi(strings.TrimSpace(string(status))) - if parseErr != nil { - return fmt.Errorf("the exec status %q could not be parsed: %w", status, parseErr) - } - - return syscall.Errno(errnoValue) -} - -func (h *execShimHandshake) closeWriteEnd() { - if h.statusW != nil { - _ = h.statusW.Close() - h.statusW = nil - } -} - -func (h *execShimHandshake) close() { - h.closeWriteEnd() - - if h.statusR != nil { - _ = h.statusR.Close() - h.statusR = nil - } -} diff --git a/internal/dcpproc/commands/fork_process_darwin_test.go b/internal/dcpproc/commands/fork_process_darwin_test.go deleted file mode 100644 index 35eab395..00000000 --- a/internal/dcpproc/commands/fork_process_darwin_test.go +++ /dev/null @@ -1,133 +0,0 @@ -//go:build darwin - -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package commands - -import ( - "bytes" - "fmt" - "os" - "os/exec" - "os/signal" - "syscall" - "testing" - "time" - - "github.com/go-logr/logr" - "github.com/stretchr/testify/require" - - "github.com/microsoft/dcp/pkg/process" - "github.com/microsoft/dcp/pkg/testutil" -) - -const ( - execShimIgnoredEndToEndEnvVar = "DCP_TEST_EXEC_SHIM_IGNORED_END_TO_END_HELPER" - execShimIgnoredEndToEndTestName = "TestExecShimPreservesIgnoredSIGUSR1Helper" -) - -func TestMain(m *testing.M) { - if len(os.Args) > 1 && os.Args[1] == ForkProcessExecCmdName { - os.Exit(runForkProcessExecTestCommand()) - } - - os.Exit(m.Run()) -} - -func runForkProcessExecTestCommand() int { - forkProcessExecCmd, commandErr := NewForkProcessExecCommand(logr.Discard()) - if commandErr != nil { - _, _ = fmt.Fprintf(os.Stderr, "could not create fork-process-exec test command: %v\n", commandErr) - return 1 - } - - forkProcessExecCmd.SetArgs(os.Args[2:]) - executeErr := forkProcessExecCmd.Execute() - if executeErr != nil { - _, _ = fmt.Fprintf(os.Stderr, "fork-process-exec test command failed: %v\n", executeErr) - return 1 - } - - return 0 -} - -func TestUseExecShimCarriesIgnoredSIGUSR1(t *testing.T) { - t.Parallel() - - childCmd := exec.Command("/bin/sh", "-c", "exit 0") - execShim, shimErr := useExecShimWithDisposition(childCmd, true) - require.NoError(t, shimErr) - require.NotNil(t, execShim) - t.Cleanup(execShim.close) - - require.Contains( - t, - childCmd.Args, - "--"+callerSIGUSR1IgnoredFlagName+"=true", - "the shim invocation should carry the caller's ignored disposition", - ) -} - -func TestExecShimPreservesIgnoredSIGUSR1ForNonGoTarget(t *testing.T) { - t.Parallel() - - runExecShimTestHelper(t, execShimIgnoredEndToEndTestName, execShimIgnoredEndToEndEnvVar) -} - -func TestExecShimPreservesIgnoredSIGUSR1Helper(t *testing.T) { - if os.Getenv(execShimIgnoredEndToEndEnvVar) == "" { - t.Skip("helper for TestExecShimPreservesIgnoredSIGUSR1ForNonGoTarget") - } - - // Only this subprocess changes its signal state; the parallel parent test remains untouched. - signal.Ignore(syscall.SIGUSR1) - ignoredByCaller, dispositionErr := process.IsSIGUSR1Ignored() - require.NoError(t, dispositionErr) - require.True(t, ignoredByCaller) - - childCmd := exec.Command("/bin/sh", "-c", `kill -USR1 $$; printf ignored`) - var childOutput bytes.Buffer - childCmd.Stdout = &childOutput - childCmd.Stderr = &childOutput - - execShim, shimErr := useExecShimWithDisposition(childCmd, true) - require.NoError(t, shimErr) - require.NotNil(t, execShim) - t.Cleanup(execShim.close) - - // The exec shim starts from this ignored disposition, then its Go runtime replaces it. - childStartErr := childCmd.Start() - require.NoError(t, childStartErr) - - childFinished := false - t.Cleanup(func() { - if !childFinished { - _ = childCmd.Process.Kill() - _ = childCmd.Wait() - } - }) - - handshakeErr := execShim.wait() - require.NoError(t, handshakeErr, "the Go exec shim should reach the non-Go target") - - childWaitErr := childCmd.Wait() - childFinished = true - require.NoError(t, childWaitErr, "the target should survive SIGUSR1; output:\n%s", childOutput.String()) - require.Equal(t, "ignored", childOutput.String()) -} - -func runExecShimTestHelper(t *testing.T, testName string, envVarName string) { - t.Helper() - - testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) - t.Cleanup(testCancel) - - helperCmd := exec.CommandContext(testCtx, os.Args[0], "-test.run=^"+testName+"$", "-test.v") - helperCmd.Env = append(os.Environ(), envVarName+"=1") - - output, runErr := helperCmd.CombinedOutput() - require.NoError(t, runErr, "helper process failed; output:\n%s", output) -} diff --git a/internal/dcpproc/commands/fork_process_exec.go b/internal/dcpproc/commands/fork_process_exec.go deleted file mode 100644 index 7bc43f50..00000000 --- a/internal/dcpproc/commands/fork_process_exec.go +++ /dev/null @@ -1,135 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package commands - -import ( - "errors" - "fmt" - "os" - "syscall" - - "github.com/go-logr/logr" - "github.com/spf13/cobra" - - cmds "github.com/microsoft/dcp/internal/commands" - "github.com/microsoft/dcp/pkg/process" -) - -const ( - // The name of the command, also used when 'fork-process' builds an invocation of it. - ForkProcessExecCmdName = "fork-process-exec" - - // The flag carrying the resolved path of the image to execute. It is passed separately from - // the arguments so that the child keeps the argv[0] the caller asked for. - execPathFlagName = "exec-path" - - // The hidden flag carrying the SIGUSR1 disposition captured before this Go process started. - callerSIGUSR1IgnoredFlagName = "caller-sigusr1-ignored" - - // The descriptor 'fork-process' passes as the only extra file, on which this command reports - // whether the exec succeeded. It is the first descriptor after the standard streams. - execStatusFd = 3 - - // Reported when the image cannot be executed, matching the shell convention for a command - // that could not be run. 'fork-process' reports the underlying errno itself, so this is only - // a fallback for anything that inspects the shim's own exit code. - execFailedExitCode = 127 -) - -var ( - execPath string - callerSIGUSR1Ignored bool -) - -// NewForkProcessExecCommand creates the 'fork-process-exec' command, which installs the clean -// SIGUSR1 disposition requested by 'fork-process' and replaces itself with the requested image. -// It is an implementation detail and is not meant to be invoked directly. -func NewForkProcessExecCommand(log logr.Logger) (*cobra.Command, error) { - forkProcessExecCmd := &cobra.Command{ - Use: ForkProcessExecCmdName + " --" + execPathFlagName + " path -- command [args...]", - Short: "Replaces this process with another program.", - Long: "Installs a clean SIGUSR1 disposition captured by 'fork-process' and then replaces this process with the requested program, keeping the same process ID. This prevents children from inheriting signal handler flags that confuse other language runtimes.", - RunE: forkProcessExec(log), - Args: validateForkProcessExecArgs, - - Hidden: true, - SilenceUsage: true, - } - - forkProcessExecCmd.Flags().StringVar(&execPath, execPathFlagName, "", "Resolved path of the program to execute") - forkProcessExecCmd.Flags().BoolVar( - &callerSIGUSR1Ignored, - callerSIGUSR1IgnoredFlagName, - false, - "Whether the caller ignored SIGUSR1 before starting the exec shim", - ) - hideDispositionFlagErr := forkProcessExecCmd.Flags().MarkHidden(callerSIGUSR1IgnoredFlagName) - if hideDispositionFlagErr != nil { - return nil, fmt.Errorf("could not hide --%s: %w", callerSIGUSR1IgnoredFlagName, hideDispositionFlagErr) - } - - return forkProcessExecCmd, nil -} - -func validateForkProcessExecArgs(_ *cobra.Command, args []string) error { - if len(trimForkProcessArgSeparator(args)) == 0 { - return fmt.Errorf("command is required") - } - - return nil -} - -func forkProcessExec(log logr.Logger) func(cmd *cobra.Command, args []string) error { - return func(_ *cobra.Command, args []string) error { - args = trimForkProcessArgSeparator(args) - - if execPath == "" { - return fmt.Errorf("--%s is required", execPathFlagName) - } - - log = log.WithName("ForkProcessExec").WithValues( - "Path", execPath, - "Args", args[1:], - ) - - // 'fork-process' waits for this descriptor to close, which is how a successful execve is - // reported, so it must not survive into the new program. It is always supplied, because - // this command is only ever started by 'fork-process'. - statusFile := os.NewFile(execStatusFd, "exec-status") - syscall.CloseOnExec(execStatusFd) - - targetEnv := os.Environ() - - prepareErr := process.PrepareSIGUSR1ForExec(callerSIGUSR1Ignored) - if prepareErr != nil { - writeForkProcessExecFailure(statusFile, prepareErr) - log.Error(prepareErr, "Could not prepare SIGUSR1 disposition for executed program") - return cmds.NewExitCodeError( - fmt.Errorf("could not prepare SIGUSR1 disposition for %q: %w", execPath, prepareErr), - execFailedExitCode, - ) - } - - // Exec must immediately follow the signal change. It only returns when it fails; on - // success this process becomes the requested program. - execErr := syscall.Exec(execPath, args, targetEnv) - - writeForkProcessExecFailure(statusFile, execErr) - - log.Error(execErr, "Could not execute the requested program") - return cmds.NewExitCodeError(fmt.Errorf("could not execute %q: %w", execPath, execErr), execFailedExitCode) - } -} - -func writeForkProcessExecFailure(statusFile *os.File, failureErr error) { - var failureErrno syscall.Errno - if !errors.As(failureErr, &failureErrno) { - failureErrno = syscall.EINVAL - } - - _, _ = fmt.Fprintf(statusFile, "%d", int(failureErrno)) - _ = statusFile.Close() -} diff --git a/internal/dcpproc/commands/fork_process_test.go b/internal/dcpproc/commands/fork_process_test.go deleted file mode 100644 index a1e64f90..00000000 --- a/internal/dcpproc/commands/fork_process_test.go +++ /dev/null @@ -1,128 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package commands - -import ( - "fmt" - "os" - "os/exec" - "strconv" - "syscall" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/microsoft/dcp/pkg/process" -) - -// Verifies that the child is redirected through the 'fork-process-exec' command on platforms -// that need it, and left alone everywhere else. The redirection is what cleans the Go runtime's -// SIGUSR1 disposition before the real program starts. -func TestUseExecShim(t *testing.T) { - t.Parallel() - - childCmd := exec.Command("sh", "-c", "exit 0") - childCmd.Env = []string{"EXISTING=value", "GODEBUG=gctrace=1,asyncpreemptoff=0,schedtrace=1000"} - originalPath := childCmd.Path - originalArgs := childCmd.Args - originalEnv := childCmd.Env - - execShim, shimErr := useExecShim(childCmd) - require.NoError(t, shimErr) - if execShim != nil { - defer execShim.close() - } - - if !process.NeedsExecSignalDispositionWorkaround() { - require.Nil(t, execShim, "no handshake is needed on this platform") - require.Equal(t, originalPath, childCmd.Path, "the command should not be redirected on this platform") - require.Equal(t, originalArgs, childCmd.Args, "the arguments should not be rewritten on this platform") - require.Equal(t, originalEnv, childCmd.Env, "the environment should not be rewritten on this platform") - return - } - - dcpPath, dcpPathErr := os.Executable() - require.NoError(t, dcpPathErr) - - ignoredByCaller, dispositionErr := process.InheritedSIGUSR1Ignored() - require.NoError(t, dispositionErr) - - expectedArgs := append( - []string{ - dcpPath, - ForkProcessExecCmdName, - "--" + execPathFlagName, - originalPath, - "--" + callerSIGUSR1IgnoredFlagName + "=" + strconv.FormatBool(ignoredByCaller), - "--", - }, - originalArgs..., - ) - - require.Equal(t, dcpPath, childCmd.Path, "the command should run the current executable") - require.Equal(t, expectedArgs, childCmd.Args, "the original program and arguments should be passed to the shim") - require.Equal(t, originalEnv, childCmd.Env, "the target environment should not be rewritten") - - require.NotNil(t, execShim, "the shim should report whether the exec succeeded") - require.Len(t, childCmd.ExtraFiles, 1, "the status descriptor should be passed to the shim") -} - -// Verifies that a command that could not be resolved is left untouched, so that starting it -// reports the original lookup failure instead of one produced by the shim. -func TestUseExecShimLeavesUnresolvedCommand(t *testing.T) { - t.Parallel() - - childCmd := exec.Command("dcp-command-that-does-not-exist") - require.Error(t, childCmd.Err, "the test requires a command that cannot be resolved") - - originalPath := childCmd.Path - originalArgs := childCmd.Args - - execShim, shimErr := useExecShim(childCmd) - require.NoError(t, shimErr) - require.Nil(t, execShim, "an unresolved command should not be redirected through the shim") - - require.Equal(t, originalPath, childCmd.Path, "an unresolved command should not be redirected") - require.Equal(t, originalArgs, childCmd.Args, "an unresolved command should not have its arguments rewritten") -} - -// Verifies that the handshake reports a successful exec, which the shim signals by closing the -// status descriptor without writing to it. -func TestExecShimHandshakeReportsSuccess(t *testing.T) { - t.Parallel() - - handshake := newTestExecShimHandshake(t) - - // Stand in for the shim: a successful execve closes the inherited descriptor. - require.NoError(t, handshake.statusW.Close()) - - require.NoError(t, handshake.wait()) -} - -// Verifies that the errno the shim reports is surfaced to the caller. Without this the caller -// would be handed the PID of a process that never became the requested program. -func TestExecShimHandshakeReportsExecFailure(t *testing.T) { - t.Parallel() - - handshake := newTestExecShimHandshake(t) - - // Stand in for the shim reporting a failed execve. - writeForkProcessExecFailure(handshake.statusW, fmt.Errorf("executing child: %w", syscall.ENOENT)) - - require.ErrorIs(t, handshake.wait(), syscall.ENOENT) -} - -func newTestExecShimHandshake(t *testing.T) *execShimHandshake { - t.Helper() - - statusR, statusW, pipeErr := os.Pipe() - require.NoError(t, pipeErr) - - handshake := &execShimHandshake{statusR: statusR, statusW: statusW} - t.Cleanup(handshake.close) - - return handshake -} diff --git a/internal/dcpproc/commands/root.go b/internal/dcpproc/commands/root.go index e9d6f84c..e7254060 100644 --- a/internal/dcpproc/commands/root.go +++ b/internal/dcpproc/commands/root.go @@ -68,12 +68,6 @@ func NewRootCmd(log *logger.Logger) (*cobra.Command, error) { rootCmd.AddCommand(cmd) } - if cmd, err = NewForkProcessExecCommand(log.Logger); err != nil { - return nil, fmt.Errorf("could not set up '%s' command: %w", ForkProcessExecCmdName, err) - } else { - rootCmd.AddCommand(cmd) - } - return rootCmd, nil } diff --git a/internal/dcpproc/fork_process_darwin_test.go b/internal/dcpproc/fork_process_darwin_test.go deleted file mode 100644 index b1014e85..00000000 --- a/internal/dcpproc/fork_process_darwin_test.go +++ /dev/null @@ -1,113 +0,0 @@ -//go:build darwin - -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package dcpproc_test - -import ( - "bytes" - "io" - "os" - "os/exec" - "path/filepath" - "strings" - "testing" - "time" - - "github.com/stretchr/testify/require" - - usvc_io "github.com/microsoft/dcp/pkg/io" - "github.com/microsoft/dcp/pkg/testutil" -) - -func TestForkProcessPreservesInheritedIgnoredSIGUSR1(t *testing.T) { - t.Parallel() - - testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) - t.Cleanup(testCancel) - - dcpProc, dcpProcErr := getDcpProcExecutablePath() - require.NoError(t, dcpProcErr) - - outputPath := filepath.Join(t.TempDir(), "target-sigusr1") - cmdArgs := forkProcessArgsForCurrentProcess( - t, - "/bin/sh", - "-c", - `kill -USR1 $$; printf ignored > "$1"`, - "sh", - outputPath, - ) - launcherArgs := append( - []string{"-c", `trap '' USR1; "$@"; status=$?; exit "$status"`, "sh", dcpProc}, - cmdArgs..., - ) - dcpProcCmd := exec.CommandContext(testCtx, "/bin/sh", launcherArgs...) - var stdout, stderr bytes.Buffer - dcpProcCmd.Stdout = &stdout - dcpProcCmd.Stderr = &stderr - - runErr := dcpProcCmd.Run() - require.NoError(t, runErr, "dcp fork-process should preserve ignored SIGUSR1; stderr: %s", stderr.String()) - _ = parseForkedPid(t, stdout.String()) - - outputFile, openErr := usvc_io.OpenFileReadOnly(outputPath) - require.NoError(t, openErr) - output, readErr := io.ReadAll(outputFile) - closeErr := outputFile.Close() - require.NoError(t, readErr) - require.NoError(t, closeErr) - require.Equal(t, "ignored", string(output)) -} - -func TestForkProcessExecShimPreservesTargetGoDebug(t *testing.T) { - t.Parallel() - - testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) - t.Cleanup(testCancel) - - dcpProc, dcpProcErr := getDcpProcExecutablePath() - require.NoError(t, dcpProcErr) - - const originalGoDebug = "asyncpreemptoff=0,tracebackancestors=7" - outputPath := filepath.Join(t.TempDir(), "target-godebug") - cmdArgs := forkProcessArgsForCurrentProcess( - t, - "/bin/sh", - "-c", - `printf '%s' "$GODEBUG" > "$1"`, - "sh", - outputPath, - ) - dcpProcCmd := exec.CommandContext(testCtx, dcpProc, cmdArgs...) - dcpProcCmd.Env = environmentWithValue(os.Environ(), "GODEBUG", originalGoDebug) - var stdout, stderr bytes.Buffer - dcpProcCmd.Stdout = &stdout - dcpProcCmd.Stderr = &stderr - - runErr := dcpProcCmd.Run() - require.NoError(t, runErr, "dcp fork-process should exit cleanly; stderr: %s", stderr.String()) - _ = parseForkedPid(t, stdout.String()) - - outputFile, openErr := usvc_io.OpenFileReadOnly(outputPath) - require.NoError(t, openErr) - output, readErr := io.ReadAll(outputFile) - closeErr := outputFile.Close() - require.NoError(t, readErr) - require.NoError(t, closeErr) - require.Equal(t, originalGoDebug, string(output), "the requested program should observe the caller's GODEBUG") -} - -func environmentWithValue(env []string, name string, value string) []string { - prefix := name + "=" - updatedEnv := make([]string, 0, len(env)+1) - for _, entry := range env { - if !strings.HasPrefix(entry, prefix) { - updatedEnv = append(updatedEnv, entry) - } - } - return append(updatedEnv, prefix+value) -} diff --git a/internal/tools/goruntimeoverlay/go-81009.patch b/internal/tools/goruntimeoverlay/go-81009.patch new file mode 100644 index 00000000..bfb8a2d4 --- /dev/null +++ b/internal/tools/goruntimeoverlay/go-81009.patch @@ -0,0 +1,17 @@ +diff --git a/src/runtime/os_darwin.go b/src/runtime/os_darwin.go +--- a/src/runtime/os_darwin.go ++++ b/src/runtime/os_darwin.go +@@ -394,7 +394,12 @@ var sigset_all = ^sigset(0) + //go:nowritebarrierrec + func setsig(i uint32, fn uintptr) { + var sa usigactiont +- sa.sa_flags = _SA_SIGINFO | _SA_ONSTACK | _SA_RESTART ++ ++ sa.sa_flags = _SA_ONSTACK | _SA_RESTART ++ // SA_SIGINFO should not be set when assigning SIG_DFL or SIG_IGN ++ if fn != _SIG_DFL && fn != _SIG_IGN { ++ sa.sa_flags |= _SA_SIGINFO ++ } + sa.sa_mask = ^uint32(0) + if fn == abi.FuncPCABIInternal(sighandler) { // abi.FuncPCABIInternal(sighandler) matches the callers in signal_unix.go + if iscgo { diff --git a/internal/tools/goruntimeoverlay/main.go b/internal/tools/goruntimeoverlay/main.go new file mode 100644 index 00000000..b2649ab0 --- /dev/null +++ b/internal/tools/goruntimeoverlay/main.go @@ -0,0 +1,230 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +package main + +import ( + "bytes" + "context" + _ "embed" + "encoding/json" + "errors" + "flag" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "runtime" + + dcpio "github.com/microsoft/dcp/pkg/io" + "github.com/microsoft/dcp/pkg/osutil" +) + +//go:embed go-81009.patch +var darwinRuntimePatch []byte + +type overlayConfig struct { + Replace map[string]string +} + +func main() { + outputDirectory := flag.String("output-dir", "", "Directory where the runtime overlay is generated") + flag.Parse() + + if *outputDirectory == "" { + _, _ = fmt.Fprintln(os.Stderr, "--output-dir is required") + os.Exit(2) + } + + generateErr := generateRuntimeOverlay(context.Background(), runtime.GOROOT(), *outputDirectory) + if generateErr != nil { + _, _ = fmt.Fprintf(os.Stderr, "could not generate Go runtime overlay: %v\n", generateErr) + os.Exit(1) + } +} + +func generateRuntimeOverlay(ctx context.Context, goRoot string, outputDirectory string) error { + runtimeSourcePath := filepath.Join(goRoot, "src", "runtime", "os_darwin.go") + runtimeSource, readErr := readFile(runtimeSourcePath) + if readErr != nil { + return fmt.Errorf("reading %q: %w", runtimeSourcePath, readErr) + } + + patchedSource, patchRequired, patchErr := applyDarwinRuntimePatch(ctx, runtimeSource) + if patchErr != nil { + return fmt.Errorf("patching %q: %w", runtimeSourcePath, patchErr) + } + + makeOutputDirectoryErr := os.MkdirAll(outputDirectory, osutil.PermissionDirectoryOthersRead) + if makeOutputDirectoryErr != nil { + return fmt.Errorf("creating output directory %q: %w", outputDirectory, makeOutputDirectoryErr) + } + + replacements := make(map[string]string, 1) + if patchRequired { + patchedSourceDirectory := filepath.Join(outputDirectory, "runtime") + makeSourceDirectoryErr := os.MkdirAll( + patchedSourceDirectory, + osutil.PermissionDirectoryOthersRead, + ) + if makeSourceDirectoryErr != nil { + return fmt.Errorf("creating patched runtime directory %q: %w", patchedSourceDirectory, makeSourceDirectoryErr) + } + + patchedSourcePath := filepath.Join(patchedSourceDirectory, "os_darwin.go") + writeSourceErr := writeFileIfChanged(patchedSourcePath, patchedSource) + if writeSourceErr != nil { + return fmt.Errorf("writing patched runtime source %q: %w", patchedSourcePath, writeSourceErr) + } + + replacements[runtimeSourcePath] = patchedSourcePath + } + + config := overlayConfig{Replace: replacements} + configContents, marshalErr := json.MarshalIndent(config, "", " ") + if marshalErr != nil { + return fmt.Errorf("encoding overlay configuration: %w", marshalErr) + } + configContents = append(configContents, '\n') + + configPath := filepath.Join(outputDirectory, "overlay.json") + writeConfigErr := writeFileIfChanged(configPath, configContents) + if writeConfigErr != nil { + return fmt.Errorf("writing overlay configuration %q: %w", configPath, writeConfigErr) + } + + return nil +} + +func applyDarwinRuntimePatch(ctx context.Context, source []byte) ([]byte, bool, error) { + stagingDirectory, createTempErr := os.MkdirTemp("", "dcp-go-runtime-overlay-") + if createTempErr != nil { + return nil, false, fmt.Errorf("creating patch staging directory: %w", createTempErr) + } + defer func() { + _ = os.RemoveAll(stagingDirectory) + }() + + stagedSourceDirectory := filepath.Join(stagingDirectory, "src", "runtime") + makeSourceDirectoryErr := os.MkdirAll(stagedSourceDirectory, osutil.PermissionDirectoryOthersRead) + if makeSourceDirectoryErr != nil { + return nil, false, fmt.Errorf("creating patch source directory: %w", makeSourceDirectoryErr) + } + + stagedSourcePath := filepath.Join(stagedSourceDirectory, "os_darwin.go") + writeSourceErr := dcpio.WriteFile( + stagedSourcePath, + source, + osutil.PermissionOwnerReadWriteOthersRead, + ) + if writeSourceErr != nil { + return nil, false, fmt.Errorf("writing patch source: %w", writeSourceErr) + } + + forwardCheckOutput, forwardCheckErr := runGitApply(ctx, stagingDirectory, false, true) + if forwardCheckErr == nil { + applyOutput, applyErr := runGitApply(ctx, stagingDirectory, false, false) + if applyErr != nil { + return nil, false, fmt.Errorf("applying upstream patch: %w: %s", applyErr, applyOutput) + } + + patchedSource, patchedSourceErr := readFile(stagedSourcePath) + if patchedSourceErr != nil { + return nil, false, fmt.Errorf("reading patched runtime source: %w", patchedSourceErr) + } + + return patchedSource, true, nil + } + if !gitApplyRejectedPatch(forwardCheckErr) { + return nil, false, fmt.Errorf( + "checking whether the upstream patch applies: %w: %s", + forwardCheckErr, + forwardCheckOutput, + ) + } + + reverseCheckOutput, reverseCheckErr := runGitApply(ctx, stagingDirectory, true, true) + if reverseCheckErr == nil { + return source, false, nil + } + if !gitApplyRejectedPatch(reverseCheckErr) { + return nil, false, fmt.Errorf( + "checking whether the upstream patch was already applied: %w: %s", + reverseCheckErr, + reverseCheckOutput, + ) + } + + return nil, false, fmt.Errorf( + "upstream patch applies neither forward nor in reverse (forward: %v: %s; reverse: %v: %s)", + forwardCheckErr, + forwardCheckOutput, + reverseCheckErr, + reverseCheckOutput, + ) +} + +func runGitApply( + ctx context.Context, + workingDirectory string, + reverse bool, + check bool, +) ([]byte, error) { + args := []string{ + "-c", "core.autocrlf=false", + "-c", "core.eol=lf", + "-C", workingDirectory, + "apply", + "--whitespace=nowarn", + } + if reverse { + args = append(args, "--reverse") + } + if check { + args = append(args, "--check") + } + args = append(args, "-") + + applyCommand := exec.CommandContext(ctx, "git", args...) + normalizedPatch := bytes.ReplaceAll(darwinRuntimePatch, []byte("\r\n"), []byte("\n")) + applyCommand.Stdin = bytes.NewReader(normalizedPatch) + output, applyErr := applyCommand.CombinedOutput() + return output, applyErr +} + +func gitApplyRejectedPatch(applyErr error) bool { + var exitErr *exec.ExitError + return errors.As(applyErr, &exitErr) +} + +func readFile(path string) ([]byte, error) { + file, openErr := dcpio.OpenFileReadOnly(path) + if openErr != nil { + return nil, openErr + } + + contents, readErr := io.ReadAll(file) + closeErr := file.Close() + if fileErr := errors.Join(readErr, closeErr); fileErr != nil { + return nil, fileErr + } + + return contents, nil +} + +func writeFileIfChanged(path string, contents []byte) error { + existingContents, readErr := readFile(path) + switch { + case readErr == nil && bytes.Equal(existingContents, contents): + return nil + case readErr == nil: + case errors.Is(readErr, os.ErrNotExist): + default: + return readErr + } + + return dcpio.WriteFile(path, contents, osutil.PermissionOwnerReadWriteOthersRead) +} diff --git a/internal/tools/goruntimeoverlay/main_test.go b/internal/tools/goruntimeoverlay/main_test.go new file mode 100644 index 00000000..cd1a2b9d --- /dev/null +++ b/internal/tools/goruntimeoverlay/main_test.go @@ -0,0 +1,149 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +package main + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + dcpio "github.com/microsoft/dcp/pkg/io" + "github.com/microsoft/dcp/pkg/osutil" +) + +const ( + unpatchedRuntimeSource = `package runtime + +func before() {} + +type sigset uint32 + +var sigset_all = ^sigset(0) + +//go:nosplit +//go:nowritebarrierrec +func setsig(i uint32, fn uintptr) { + var sa usigactiont + sa.sa_flags = _SA_SIGINFO | _SA_ONSTACK | _SA_RESTART + sa.sa_mask = ^uint32(0) + if fn == abi.FuncPCABIInternal(sighandler) { // abi.FuncPCABIInternal(sighandler) matches the callers in signal_unix.go + if iscgo { + } + } +} + +func after() {} +` + patchedRuntimeSource = `package runtime + +func before() {} + +type sigset uint32 + +var sigset_all = ^sigset(0) + +//go:nosplit +//go:nowritebarrierrec +func setsig(i uint32, fn uintptr) { + var sa usigactiont + + sa.sa_flags = _SA_ONSTACK | _SA_RESTART + // SA_SIGINFO should not be set when assigning SIG_DFL or SIG_IGN + if fn != _SIG_DFL && fn != _SIG_IGN { + sa.sa_flags |= _SA_SIGINFO + } + sa.sa_mask = ^uint32(0) + if fn == abi.FuncPCABIInternal(sighandler) { // abi.FuncPCABIInternal(sighandler) matches the callers in signal_unix.go + if iscgo { + } + } +} + +func after() {} +` +) + +func TestGenerateRuntimeOverlay(t *testing.T) { + t.Parallel() + + goRoot := t.TempDir() + runtimeDirectory := filepath.Join(goRoot, "src", "runtime") + require.NoError(t, os.MkdirAll(runtimeDirectory, osutil.PermissionDirectoryOthersRead)) + + runtimeSourcePath := filepath.Join(runtimeDirectory, "os_darwin.go") + runtimeSource := []byte(unpatchedRuntimeSource) + require.NoError(t, dcpio.WriteFile( + runtimeSourcePath, + runtimeSource, + osutil.PermissionOwnerReadWriteOthersRead, + )) + + outputDirectory := filepath.Join(t.TempDir(), "overlay") + require.NoError(t, generateRuntimeOverlay(context.Background(), goRoot, outputDirectory)) + + patchedSourcePath := filepath.Join(outputDirectory, "runtime", "os_darwin.go") + patchedSource, patchedSourceErr := readFile(patchedSourcePath) + require.NoError(t, patchedSourceErr) + require.Equal(t, patchedRuntimeSource, string(patchedSource)) + + configContents, configReadErr := readFile(filepath.Join(outputDirectory, "overlay.json")) + require.NoError(t, configReadErr) + + var config overlayConfig + require.NoError(t, json.Unmarshal(configContents, &config)) + require.Equal(t, map[string]string{runtimeSourcePath: patchedSourcePath}, config.Replace) +} + +// Remove this test with the runtime overlay once DCP requires a Go release containing golang/go#81009. +func TestGenerateRuntimeOverlayIsEmptyWhenToolchainContainsFix(t *testing.T) { + t.Parallel() + + goRoot := t.TempDir() + runtimeDirectory := filepath.Join(goRoot, "src", "runtime") + require.NoError(t, os.MkdirAll(runtimeDirectory, osutil.PermissionDirectoryOthersRead)) + + runtimeSourcePath := filepath.Join(runtimeDirectory, "os_darwin.go") + require.NoError(t, dcpio.WriteFile( + runtimeSourcePath, + []byte(patchedRuntimeSource), + osutil.PermissionOwnerReadWriteOthersRead, + )) + + outputDirectory := filepath.Join(t.TempDir(), "overlay") + require.NoError(t, generateRuntimeOverlay(context.Background(), goRoot, outputDirectory)) + + configContents, configReadErr := readFile(filepath.Join(outputDirectory, "overlay.json")) + require.NoError(t, configReadErr) + + var config overlayConfig + require.NoError(t, json.Unmarshal(configContents, &config)) + require.Empty(t, config.Replace) +} + +// Remove this test with the runtime overlay once DCP requires a Go release containing golang/go#81009. +func TestGenerateRuntimeOverlayRejectsUnexpectedSource(t *testing.T) { + t.Parallel() + + goRoot := t.TempDir() + runtimeDirectory := filepath.Join(goRoot, "src", "runtime") + require.NoError(t, os.MkdirAll(runtimeDirectory, osutil.PermissionDirectoryOthersRead)) + + runtimeSourcePath := filepath.Join(runtimeDirectory, "os_darwin.go") + require.NoError(t, dcpio.WriteFile( + runtimeSourcePath, + []byte("package runtime\n"), + osutil.PermissionOwnerReadWriteOthersRead, + )) + + outputDirectory := filepath.Join(t.TempDir(), "overlay") + generateErr := generateRuntimeOverlay(context.Background(), goRoot, outputDirectory) + + require.ErrorContains(t, generateErr, "upstream patch applies neither forward nor in reverse") +} diff --git a/pkg/process/os_executor_darwin_test.go b/pkg/process/os_executor_darwin_test.go new file mode 100644 index 00000000..e924f657 --- /dev/null +++ b/pkg/process/os_executor_darwin_test.go @@ -0,0 +1,52 @@ +//go:build darwin + +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +package process_test + +import ( + "bytes" + "os/exec" + "testing" + "time" + + int_testutil "github.com/microsoft/dcp/internal/testutil" + "github.com/microsoft/dcp/pkg/testutil" + "github.com/stretchr/testify/require" +) + +// Remove this test with the runtime overlay once DCP requires a Go release containing golang/go#81009. +func TestStartProcessChildDoesNotInheritSIGINFOWithDefaultHandler(t *testing.T) { + t.Parallel() + + signalDispositionTool, toolPathErr := int_testutil.GetTestToolPath("signal-disposition") + require.NoError( + t, + toolPathErr, + "could not locate signal-disposition test tool (did you run `make test-prereqs`?)", + ) + + testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) + defer testCancel() + + testDeadline, haveTestDeadline := testCtx.Deadline() + require.True(t, haveTestDeadline, "signal disposition test context should have a deadline") + + launcherTimeout := time.Until(testDeadline) + require.Positive(t, launcherTimeout, "signal disposition test deadline should be in the future") + + launcherCmd := exec.CommandContext( + testCtx, + signalDispositionTool, + "--timeout", launcherTimeout.String(), + ) + var launcherOutput bytes.Buffer + launcherCmd.Stdout = &launcherOutput + launcherCmd.Stderr = &launcherOutput + + runErr := launcherCmd.Run() + require.NoError(t, runErr, "signal disposition launcher failed:\n%s", launcherOutput.String()) +} diff --git a/pkg/process/signal_disposition_darwin.go b/pkg/process/signal_disposition_darwin.go deleted file mode 100644 index c3eec72f..00000000 --- a/pkg/process/signal_disposition_darwin.go +++ /dev/null @@ -1,152 +0,0 @@ -//go:build darwin - -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package process - -import ( - "fmt" - "os" - "syscall" - "unsafe" - - "golang.org/x/sys/unix" -) - -const ( - // Darwin's NSIG. Valid signal numbers are 1 through darwinNumSignals-1. - darwinNumSignals = 32 - - // SIG_DFL and SIG_IGN, which the syscall package does not define. - darwinSigDfl = uintptr(0) - darwinSigIgn = uintptr(1) -) - -// InheritedSIGUSR1Ignored reports whether SIGUSR1 was ignored when this process started. -// -// The Go runtime replaces an inherited SIG_IGN disposition for SIGUSR1 during startup, so the -// original state is recovered from the live launcher process that supplied it. -func InheritedSIGUSR1Ignored() (bool, error) { - parentPID := os.Getppid() - if parentPID <= 0 { - return false, fmt.Errorf("invalid launcher process ID %d", parentPID) - } - - parentInfo, parentInfoErr := unix.SysctlKinfoProc("kern.proc.pid", parentPID) - if parentInfoErr != nil { - return false, fmt.Errorf("reading launcher process %d signal dispositions: %w", parentPID, parentInfoErr) - } - if parentInfo == nil { - return false, fmt.Errorf("launcher process %d returned no process information", parentPID) - } - if parentInfo.Proc.P_pid != int32(parentPID) { - return false, fmt.Errorf( - "launcher process query returned process %d instead of %d", - parentInfo.Proc.P_pid, - parentPID, - ) - } - - signalNumber := uint32(syscall.SIGUSR1) - if signalNumber == 0 || signalNumber >= darwinNumSignals { - return false, fmt.Errorf("SIGUSR1 number %d cannot be represented in Darwin sigset_t", signalNumber) - } - - // Darwin's sigmask macro assigns signal N to bit N-1. - signalMask := uint32(1) << (signalNumber - 1) - return parentInfo.Proc.P_sigignore&signalMask != 0, nil -} - -// darwinSigactionNew mirrors Darwin's `struct __sigaction`, which is the layout the -// sigaction(2) system call expects for the new disposition. It differs from the userspace -// `struct sigaction` by the sa_tramp field that libc fills in with the signal trampoline. -// A nil trampoline is only safe when the handler is SIG_DFL or SIG_IGN, because the kernel -// stores the trampoline but never invokes it in those cases. -type darwinSigactionNew struct { - handler uintptr - tramp uintptr - mask uint32 - flags int32 -} - -// darwinSigactionOld mirrors Darwin's userspace `struct sigaction`, which is the layout the -// sigaction(2) system call uses when reporting the previous disposition. -type darwinSigactionOld struct { - handler uintptr - mask uint32 - flags int32 -} - -// NeedsExecSignalDispositionWorkaround reports whether an exec'd child can inherit a SIGUSR1 -// disposition that is incompatible with other language runtimes. -// -// Darwin's execve(2) resets signal handlers to SIG_DFL but preserves sa_flags. Linux clears -// sa_flags along with the handler, and Windows has no signal dispositions at all. -func NeedsExecSignalDispositionWorkaround() bool { - return true -} - -// IsSIGUSR1Ignored reports whether SIGUSR1 currently has the SIG_IGN disposition. -func IsSIGUSR1Ignored() (bool, error) { - current, currentErr := signalDisposition(int(syscall.SIGUSR1)) - if currentErr != nil { - return false, fmt.Errorf("reading SIGUSR1 disposition: %w", currentErr) - } - - return current.handler == darwinSigIgn, nil -} - -// PrepareSIGUSR1ForExec gives SIGUSR1 the requested disposition with no trampoline, flags, or -// mask. The requested disposition must be captured before starting this Go process because the -// Go runtime may replace the inherited disposition during startup. -// -// Affected Go releases install handlers with SA_SIGINFO|SA_ONSTACK|SA_RESTART and restore them -// to SIG_DFL before exec without clearing those flags. Because Darwin's execve(2) preserves -// sa_flags, children can inherit SIG_DFL together with SA_SIGINFO. .NET misinterprets that as a -// handler being present and jumps to address zero when it first uses SIGUSR1 to suspend threads -// for a garbage collection. -// -// Resetting in a process that then forks is not sufficient, because the Go runtime restores its -// own dispositions in the forked child before it reaches execve. The reset has to happen in the -// process that calls exec, which is what the 'fork-process-exec' command exists to do. This -// workaround can be removed once DCP requires a Go release containing golang/go#81009. -func PrepareSIGUSR1ForExec(ignoredByCaller bool) error { - targetHandler := darwinSigDfl - if ignoredByCaller { - targetHandler = darwinSigIgn - } - - act := darwinSigactionNew{ - handler: targetHandler, - tramp: 0, - mask: 0, - flags: 0, - } - - if setErr := setSignalDisposition(int(syscall.SIGUSR1), &act); setErr != nil { - return fmt.Errorf("setting SIGUSR1 disposition for exec: %w", setErr) - } - - return nil -} - -func setSignalDisposition(sig int, act *darwinSigactionNew) error { - if _, _, errno := syscall.Syscall(syscall.SYS_SIGACTION, uintptr(sig), uintptr(unsafe.Pointer(act)), 0); errno != 0 { - return errno - } - - return nil -} - -// signalDisposition reports the current handler and flags for a signal. -func signalDisposition(sig int) (darwinSigactionOld, error) { - var current darwinSigactionOld - if _, _, errno := syscall.Syscall(syscall.SYS_SIGACTION, uintptr(sig), 0, uintptr(unsafe.Pointer(¤t))); errno != 0 { - return current, errno - } - - return current, nil -} diff --git a/pkg/process/signal_disposition_darwin_test.go b/pkg/process/signal_disposition_darwin_test.go deleted file mode 100644 index a9b6cc9e..00000000 --- a/pkg/process/signal_disposition_darwin_test.go +++ /dev/null @@ -1,171 +0,0 @@ -//go:build darwin - -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package process - -import ( - "os" - "os/exec" - "os/signal" - "syscall" - "testing" - "time" - - "github.com/stretchr/testify/require" - - "github.com/microsoft/dcp/pkg/testutil" -) - -const ( - prepareSIGUSR1ForExecHelperEnvVar = "DCP_TEST_PREPARE_SIGUSR1_FOR_EXEC_HELPER" - prepareSIGUSR1DefaultMode = "default" - prepareSIGUSR1IgnoredMode = "ignored" - darwinTestSARestart int32 = 0x2 - dirtySignalMask uint32 = 1 -) - -func TestPrepareSIGUSR1ForExecUsesDefaultDisposition(t *testing.T) { - t.Parallel() - - runPrepareSIGUSR1ForExecHelper(t, prepareSIGUSR1DefaultMode) -} - -func TestPrepareSIGUSR1ForExecUsesIgnoredDisposition(t *testing.T) { - t.Parallel() - - runPrepareSIGUSR1ForExecHelper(t, prepareSIGUSR1IgnoredMode) -} - -func runPrepareSIGUSR1ForExecHelper(t *testing.T, mode string) { - t.Helper() - - testCtx, testCancel := testutil.GetTestContext(t, 30*time.Second) - t.Cleanup(testCancel) - - helper := exec.CommandContext(testCtx, os.Args[0], "-test.run=^TestPrepareSIGUSR1ForExecHelper$", "-test.v") - helper.Env = append(os.Environ(), prepareSIGUSR1ForExecHelperEnvVar+"="+mode) - - output, runErr := helper.CombinedOutput() - require.NoError(t, runErr, "helper process failed; output:\n%s", output) -} - -func TestPrepareSIGUSR1ForExecHelper(t *testing.T) { - mode := os.Getenv(prepareSIGUSR1ForExecHelperEnvVar) - if mode == "" { - t.Skip("helper for TestPrepareSIGUSR1ForExec") - } - - switch mode { - case prepareSIGUSR1DefaultMode: - testPrepareSIGUSR1ForExecDefault(t) - case prepareSIGUSR1IgnoredMode: - testPrepareSIGUSR1ForExecIgnored(t) - default: - t.Fatalf("unknown helper mode %q", mode) - } -} - -func testPrepareSIGUSR1ForExecDefault(t *testing.T) { - t.Helper() - - initiallyIgnored, ignoredErr := IsSIGUSR1Ignored() - require.NoError(t, ignoredErr) - require.False(t, initiallyIgnored, "the Go test process should not start with SIGUSR1 ignored") - - // The requested target disposition, rather than the shim's current disposition, must win. - setDirtySIGUSR1Disposition(t, darwinSigIgn) - before := readSignalDispositions(t) - beforeSIGUSR1 := before[int(syscall.SIGUSR1)] - require.Equal(t, darwinSigIgn, beforeSIGUSR1.handler) - require.NotZero(t, beforeSIGUSR1.flags) - require.NotZero(t, beforeSIGUSR1.mask) - - require.NoError(t, PrepareSIGUSR1ForExec(false)) - - assertPreparedSIGUSR1Disposition(t, before, darwinSigDfl) -} - -func testPrepareSIGUSR1ForExecIgnored(t *testing.T) { - t.Helper() - - signal.Ignore(syscall.SIGUSR1) - ignored, ignoredErr := IsSIGUSR1Ignored() - require.NoError(t, ignoredErr) - require.True(t, ignored, "SIGUSR1 should be reported as ignored") - - // The requested target disposition, rather than the shim's current disposition, must win. - setDirtySIGUSR1Disposition(t, darwinSigDfl) - before := readSignalDispositions(t) - beforeSIGUSR1 := before[int(syscall.SIGUSR1)] - require.Equal(t, darwinSigDfl, beforeSIGUSR1.handler) - require.NotZero(t, beforeSIGUSR1.flags) - require.NotZero(t, beforeSIGUSR1.mask) - - require.NoError(t, PrepareSIGUSR1ForExec(true)) - - assertPreparedSIGUSR1Disposition(t, before, darwinSigIgn) -} - -func setDirtySIGUSR1Disposition(t *testing.T, handler uintptr) { - t.Helper() - - dirtyAction := darwinSigactionNew{ - handler: handler, - mask: dirtySignalMask, - flags: darwinTestSARestart, - } - setErr := setSignalDisposition(int(syscall.SIGUSR1), &dirtyAction) - require.NoError(t, setErr) -} - -func assertPreparedSIGUSR1Disposition( - t *testing.T, - before map[int]darwinSigactionOld, - expectedHandler uintptr, -) { - t.Helper() - - after := readSignalDispositions(t) - afterSIGUSR1 := after[int(syscall.SIGUSR1)] - require.Equal(t, expectedHandler, afterSIGUSR1.handler, "SIGUSR1 should have the requested handler") - require.Zero(t, afterSIGUSR1.flags, "SIGUSR1 should have no flags") - require.Zero(t, afterSIGUSR1.mask, "SIGUSR1 should have an empty mask") - require.Equal( - t, - before[int(syscall.SIGURG)], - after[int(syscall.SIGURG)], - "SIGURG disposition should remain unchanged", - ) - - for sig := 1; sig < darwinNumSignals; sig++ { - if sig == int(syscall.SIGKILL) || - sig == int(syscall.SIGSTOP) || - sig == int(syscall.SIGUSR1) || - sig == int(syscall.SIGURG) { - continue - } - - require.Equal(t, before[sig], after[sig], "signal %d disposition should remain unchanged", sig) - } -} - -func readSignalDispositions(t *testing.T) map[int]darwinSigactionOld { - t.Helper() - - dispositions := make(map[int]darwinSigactionOld, darwinNumSignals-3) - for sig := 1; sig < darwinNumSignals; sig++ { - if sig == int(syscall.SIGKILL) || sig == int(syscall.SIGSTOP) { - continue - } - - disposition, dispositionErr := signalDisposition(sig) - require.NoError(t, dispositionErr, "signal %d disposition should be readable", sig) - dispositions[sig] = disposition - } - - return dispositions -} diff --git a/pkg/process/signal_disposition_other.go b/pkg/process/signal_disposition_other.go deleted file mode 100644 index 30ab51c3..00000000 --- a/pkg/process/signal_disposition_other.go +++ /dev/null @@ -1,36 +0,0 @@ -//go:build !darwin - -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See LICENSE in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -package process - -// NeedsExecSignalDispositionWorkaround reports whether an exec'd child can inherit a SIGUSR1 -// disposition that is incompatible with other language runtimes. -// -// Only Darwin is affected: its execve(2) resets signal handlers to SIG_DFL but preserves -// sa_flags. Linux clears sa_flags along with the handler, and Windows has no signal dispositions -// at all. -func NeedsExecSignalDispositionWorkaround() bool { - return false -} - -// InheritedSIGUSR1Ignored reports whether SIGUSR1 was ignored when this process started. It -// reports false on platforms that do not need the Darwin exec signal disposition workaround. -func InheritedSIGUSR1Ignored() (bool, error) { - return false, nil -} - -// IsSIGUSR1Ignored reports whether SIGUSR1 currently has the SIG_IGN disposition. It reports -// false on platforms that do not need the Darwin exec signal disposition workaround. -func IsSIGUSR1Ignored() (bool, error) { - return false, nil -} - -// PrepareSIGUSR1ForExec gives SIGUSR1 a disposition that is safe for an exec'd child. It is a -// no-op wherever NeedsExecSignalDispositionWorkaround reports false. -func PrepareSIGUSR1ForExec(_ bool) error { - return nil -} diff --git a/test/signaldisposition/main.go b/test/signaldisposition/main.go new file mode 100644 index 00000000..867c3ea0 --- /dev/null +++ b/test/signaldisposition/main.go @@ -0,0 +1,117 @@ +//go:build darwin && cgo + +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +package main + +/* +#include + +static int initial_handler_is_default; +static int initial_has_siginfo; +static int initial_flags; + +__attribute__((constructor)) +static void capture_initial_sigusr1(void) { + struct sigaction action; + if (sigaction(SIGUSR1, NULL, &action) == 0) { + initial_handler_is_default = action.sa_handler == SIG_DFL; + initial_has_siginfo = (action.sa_flags & SA_SIGINFO) != 0; + initial_flags = action.sa_flags; + } +} + +static int get_initial_handler_is_default(void) { + return initial_handler_is_default; +} + +static int get_initial_has_siginfo(void) { + return initial_has_siginfo; +} + +static int get_initial_flags(void) { + return initial_flags; +} +*/ +import "C" + +import ( + "context" + "flag" + "fmt" + "os" + "os/exec" + "time" + + "github.com/go-logr/logr" + + "github.com/microsoft/dcp/pkg/process" +) + +const childArgument = "child" + +func main() { + if len(os.Args) == 2 && os.Args[1] == childArgument { + validateChildSignalDisposition() + return + } + + launcherFlags := flag.NewFlagSet("signal-disposition", flag.ContinueOnError) + launcherTimeout := launcherFlags.Duration("timeout", 30*time.Second, "Maximum time to wait for the child") + if parseErr := launcherFlags.Parse(os.Args[1:]); parseErr != nil { + os.Exit(2) + } + if launcherFlags.NArg() != 0 { + _, _ = fmt.Fprintf(os.Stderr, "unexpected arguments: %q\n", launcherFlags.Args()) + os.Exit(2) + } + + runLauncher(*launcherTimeout) +} + +func validateChildSignalDisposition() { + handlerIsDefault := C.get_initial_handler_is_default() != 0 + hasSIGINFO := C.get_initial_has_siginfo() != 0 + flags := uint32(C.get_initial_flags()) + + if !handlerIsDefault || hasSIGINFO { + _, _ = fmt.Fprintf( + os.Stderr, + "SIGUSR1 at process startup: default=%t flags=%#x\n", + handlerIsDefault, + flags, + ) + os.Exit(1) + } +} + +func runLauncher(timeout time.Duration) { + executablePath, executablePathErr := os.Executable() + if executablePathErr != nil { + _, _ = fmt.Fprintf(os.Stderr, "could not determine signal disposition executable path: %v\n", executablePathErr) + os.Exit(1) + } + + runCtx, runCancel := context.WithTimeout(context.Background(), timeout) + defer runCancel() + + childCmd := exec.Command(executablePath, childArgument) + childCmd.Stdout = os.Stdout + childCmd.Stderr = os.Stderr + + executor := process.NewOSExecutor(logr.Discard()) + defer executor.Dispose() + + exitCode, runErr := process.RunToCompletion(runCtx, executor, childCmd) + if runErr != nil { + _, _ = fmt.Fprintf(os.Stderr, "could not run signal disposition child: %v\n", runErr) + os.Exit(1) + } + if exitCode != 0 { + _, _ = fmt.Fprintf(os.Stderr, "signal disposition child exited with code %d\n", exitCode) + os.Exit(1) + } +}