Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime/cgo: cgo traceback routine can use value that msan thinks is uninitialized #47543

Closed
ianlancetaylor opened this issue Aug 5, 2021 · 1 comment
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

When building with -msan to use the C memory sanitizer, the cgo traceback routine set by runtime.SetCgoTraceback can use a value that msan thinks is uninitialized. This can happen because the C function x_cgo_callers, in runtime/cgo, is called directly by cgoSigtramp. The arguments are passed in registers as is normal for C. cgoSigtramp is written in assembly, and as such does not notify MSAN what it is doing. The effect is that if the signal handler is invoked at a point in the program where one of the argument registers holds uninitialized memory, MSAN will think that x_cgo_callers is using an argument register that holds uninitialized memory. To be precise, the first three argument registers are set by the signal handler, and it so happens that MSAN doesn't care about the way that x_cgo_callers uses its fourth or sixth argument, but MSAN does care about the use of the fifth argument, cgoCallers. If the fifth argument register is uninitialized when the signal handler is invoked, x_cgo_callers can cause the arg.Buf field of the cgo traceback argument to be uninitialized. If the cgo traceback function tries to use arg.Buf, as it almost certainly will, then MSAN will report a use of uninitialized memory.

This is a fairly complex scenario, but below is a test case that reproduces it reliably when built with

CC=clang CGO_CFLAGS="-fsanitize=memory" go build

Fortunately the fix is straightforward and safe. I will send a CL.

package main

/*
#include <pthread.h>
#include <signal.h>
#include <stdint.h>

#include <sanitizer/msan_interface.h>

// cgoTracebackArg is the type of the argument passed to msanGoTraceback.
struct cgoTracebackArg {
	uintptr_t context;
	uintptr_t sigContext;
	uintptr_t* buf;
	uintptr_t max;
};

// msanGoTraceback is registered as the cgo traceback function.
// This will be called when a signal occurs.
void msanGoTraceback(void* parg) {
	struct cgoTracebackArg* arg = (struct cgoTracebackArg*)(parg);
        arg->buf[0] = 0;
}

// msanGoWait will be called with all registers undefined as far as
// msan is concerned. It just waits for a signal.
// Because the registers are msan-undefined, the signal handler will
// be invoked with all registers msan-undefined.
__attribute__((noinline))
void msanGoWait(unsigned long a1, unsigned long a2, unsigned long a3, unsigned long a4, unsigned long a5, unsigned long a6) {
	sigset_t mask;

	sigemptyset(&mask);
        sigsuspend(&mask);
}

// msanGoSignalThread is the thread ID of the msanGoLoop thread.
static pthread_t msanGoSignalThread;

// msanGoSignalThreadSet is used to record that msanGoSignalThread
// has been initialized. This is accessed atomically.
static int32_t msanGoSignalThreadSet;

// uninit is explicitly poisoned, so that we can make all registers
// undefined by calling msanGoWait.
static unsigned long uninit;

// msanGoLoop loops calling msanGoWait, with the arguments passed
// such that msan thinks that they are undefined. msan permits
// undefined values to be used as long as they are not used to
// for conditionals or for memory access.
void msanGoLoop() {
	int i;

	msanGoSignalThread = pthread_self();
        __atomic_store_n(&msanGoSignalThreadSet, 1, __ATOMIC_SEQ_CST);

	// Force uninit to be undefined for msan.
	__msan_poison(&uninit, sizeof uninit);
	for (i = 0; i < 100; i++) {
		msanGoWait(uninit, uninit, uninit, uninit, uninit, uninit);
        }
}

// msanGoReady returns whether msanGoSignalThread is set.
int msanGoReady() {
	return __atomic_load_n(&msanGoSignalThreadSet, __ATOMIC_SEQ_CST) != 0;
}

// msanGoSendSignal sends a signal to the msanGoLoop thread.
void msanGoSendSignal() {
	pthread_kill(msanGoSignalThread, SIGWINCH);
}
*/
import "C"

import (
	"runtime"
	"time"
)

func main() {
	runtime.SetCgoTraceback(0, C.msanGoTraceback, nil, nil)

	c := make(chan bool)
	go func() {
		defer func() { c <- true }()
		C.msanGoLoop()
	}()

	for C.msanGoReady() == 0 {
		time.Sleep(time.Microsecond)
	}

loop:
	for {
		select {
		case <-c:
			break loop
		default:
			C.msanGoSendSignal()
			time.Sleep(time.Microsecond)
		}
	}
}
@ianlancetaylor ianlancetaylor added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Aug 5, 2021
@ianlancetaylor ianlancetaylor added this to the Go1.17 milestone Aug 5, 2021
@gopherbot
Copy link

Change https://golang.org/cl/339902 mentions this issue: runtime/cgo: when using msan explicitly unpoison cgoCallers

@golang golang locked and limited conversation to collaborators Aug 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

2 participants