diff --git a/tailscale.go b/tailscale.go index ce7cb74..627f305 100644 --- a/tailscale.go +++ b/tailscale.go @@ -586,7 +586,9 @@ func TsnetSetLogFD(sd, fd C.int) C.int { s.s.Logf = logger.Discard return 0 } - f := os.NewFile(uintptr(fd), "logfd") + // Write to fd directly rather than through os.NewFile: an *os.File + // closes its fd when garbage collected, and fd belongs to the caller. + f := fdWriter(fd) s.s.Logf = func(format string, args ...any) { fmt.Fprintf(f, format, args...) fmt.Fprintf(f, "\n") @@ -594,6 +596,27 @@ func TsnetSetLogFD(sd, fd C.int) C.int { return 0 } +// fdWriter is an io.Writer for a file descriptor it does not own. +type fdWriter int + +func (w fdWriter) Write(b []byte) (int, error) { + var n int + for n < len(b) { + m, err := syscall.Write(int(w), b[n:]) + if err == syscall.EINTR { + continue + } + if err != nil { + return n, err + } + if m == 0 { + return n, io.ErrShortWrite + } + n += m + } + return n, nil +} + //export TsnetLoopback func TsnetLoopback(sd C.int, addrOut *C.char, addrLen C.size_t, proxyOut *C.char, localOut *C.char) C.int { // Panic here to ensure we always leave the out values NUL-terminated. diff --git a/tailscale_test.go b/tailscale_test.go index ee8e822..91e08c9 100644 --- a/tailscale_test.go +++ b/tailscale_test.go @@ -66,6 +66,10 @@ func TestGetRemoteAddr(t *testing.T) { tsnetctest.RunTestGetRemoteAddr(t) } +func TestSetLogFD(t *testing.T) { + tsnetctest.RunTestSetLogFD(t) +} + func TestExtractIP(t *testing.T) { ipv4 := "1.23.33.4:12343" ipv6 := "[1::2234::34fc::44]:56576" diff --git a/tsnetctest/tsnetctest.go b/tsnetctest/tsnetctest.go index ed4e5cc..41bbc3c 100644 --- a/tsnetctest/tsnetctest.go +++ b/tsnetctest/tsnetctest.go @@ -294,9 +294,12 @@ import ( "net/http/httptest" "os" "path/filepath" + "runtime" + "syscall" "testing" "time" + "golang.org/x/sys/unix" "tailscale.com/net/netns" "tailscale.com/tstest/integration" "tailscale.com/tstest/integration/testcontrol" @@ -404,3 +407,37 @@ func RunTestGetRemoteAddr(t *testing.T) { t.Fatal(C.GoString(C.err)) } } + +// RunTestSetLogFD checks that libtailscale leaves the fd passed to +// tailscale_set_logfd open after the logger is replaced and the +// server is closed: the fd belongs to the caller. +func RunTestSetLogFD(t *testing.T) { + var p [2]int + if err := syscall.Pipe(p[:]); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + syscall.Close(p[0]) + syscall.Close(p[1]) + }) + + sd := C.tailscale_new() + if C.tailscale_set_logfd(sd, C.int(p[1])) != 0 { + t.Fatal("tailscale_set_logfd failed") + } + if C.tailscale_set_logfd(sd, -1) != 0 { + t.Fatal("tailscale_set_logfd(-1) failed") + } + if C.tailscale_close(sd) != 0 { + t.Fatal("tailscale_close failed") + } + + // Give any finalizer holding the fd a chance to run. + for i := 0; i < 10; i++ { + runtime.GC() + time.Sleep(10 * time.Millisecond) + } + if _, err := unix.FcntlInt(uintptr(p[1]), unix.F_GETFD, 0); err != nil { + t.Fatalf("log fd %d was closed by libtailscale: %v", p[1], err) + } +}