From 450189162e15c813713b5c3a5208bdb6380c17f8 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Thu, 3 Dec 2020 13:40:02 +0100 Subject: ringlogger: hook into global panic writer This is a grotesque hack, and hopefully upstream Go will provide a nicer way of doing this, but already it seems quite adept at catching panics. See https://github.com/golang/go/issues/42888 for more info. This requires us to rewrite the ringlogger path to avoid all allocations. Signed-off-by: Jason A. Donenfeld --- ...d-link-recognize-arm-header-of-PE-objects.patch | 2 +- ...al-with-ADDR32NB-relocations-the-same-way.patch | 2 +- ...cmd-link-ignore-SEH-marking-on-PE-objects.patch | 2 +- ...-do-not-mark-resource-section-as-writable.patch | 2 +- ...cmd-link-handle-grouped-resource-sections.patch | 2 +- ...ase-branch.go1.15-runtime-detect-services.patch | 2 +- ...me-do-not-explicitly-exit-on-ctrl-handler.patch | 2 +- ...ow-callback-functions-with-up-to-8-argume.patch | 2 +- ...-CreateWaitableTimerEx-to-implement-uslee.patch | 2 +- ...ow-for-usleep2HighRes-to-run-without-TLS-.patch | 2 +- ...windows-arm-is-all-pie-so-mark-it-as-such.patch | 2 +- ...ust-address-calculation-in-identifying-ab.patch | 2 +- ...le-do-not-assume-TST-and-TEQ-set-V-on-arm.patch | 2 +- ...-runtime-make-write-into-function-pointer.patch | 32 ++++++++++++++++++++ manager/interfacecleanup.go | 2 -- manager/ipc_server.go | 1 - manager/service.go | 16 ---------- manager/tunneltracker.go | 2 -- manager/updatestate.go | 2 -- ringlogger/global.go | 35 ++++++++++++++++++++++ ringlogger/ringlogger.go | 29 ++++++++++++------ 21 files changed, 100 insertions(+), 45 deletions(-) create mode 100644 go-patches/0014-runtime-make-write-into-function-pointer.patch diff --git a/go-patches/0001-cmd-link-recognize-arm-header-of-PE-objects.patch b/go-patches/0001-cmd-link-recognize-arm-header-of-PE-objects.patch index 514103e7..19fdd04a 100644 --- a/go-patches/0001-cmd-link-recognize-arm-header-of-PE-objects.patch +++ b/go-patches/0001-cmd-link-recognize-arm-header-of-PE-objects.patch @@ -1,7 +1,7 @@ From fde4a13eb5eba28ac546b10752b38a80f389cc5a Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sun, 8 Nov 2020 02:48:09 +0100 -Subject: [PATCH 01/13] cmd/link: recognize arm header of PE objects +Subject: [PATCH 01/14] cmd/link: recognize arm header of PE objects The linker recognizes headers for 386 and amd64 PE objects, but not arm objects. This is easily overlooked, since its the same as the 386 header diff --git a/go-patches/0002-cmd-link-deal-with-ADDR32NB-relocations-the-same-way.patch b/go-patches/0002-cmd-link-deal-with-ADDR32NB-relocations-the-same-way.patch index 9c4bf226..e260ccd8 100644 --- a/go-patches/0002-cmd-link-deal-with-ADDR32NB-relocations-the-same-way.patch +++ b/go-patches/0002-cmd-link-deal-with-ADDR32NB-relocations-the-same-way.patch @@ -1,7 +1,7 @@ From a0a59fc25f4d8aef3c7bce38c84f04c504745f0e Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sun, 8 Nov 2020 03:09:42 +0100 -Subject: [PATCH 02/13] cmd/link: deal with ADDR32NB relocations the same way +Subject: [PATCH 02/14] cmd/link: deal with ADDR32NB relocations the same way as ADDR32 on arm As far as I can tell, the addend is the same for both of these, and in diff --git a/go-patches/0003-cmd-link-ignore-SEH-marking-on-PE-objects.patch b/go-patches/0003-cmd-link-ignore-SEH-marking-on-PE-objects.patch index 02d5ff31..e5c86f88 100644 --- a/go-patches/0003-cmd-link-ignore-SEH-marking-on-PE-objects.patch +++ b/go-patches/0003-cmd-link-ignore-SEH-marking-on-PE-objects.patch @@ -1,7 +1,7 @@ From e8142ab5e3b3a513683a8e3792e6197644547981 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sun, 8 Nov 2020 03:20:36 +0100 -Subject: [PATCH 03/13] cmd/link: ignore SEH marking on PE objects +Subject: [PATCH 03/14] cmd/link: ignore SEH marking on PE objects Microsoft's linker looks at whether all input objects have an empty section called @feat.00. If all of them do, then it enables SEH; diff --git a/go-patches/0004-cmd-link-do-not-mark-resource-section-as-writable.patch b/go-patches/0004-cmd-link-do-not-mark-resource-section-as-writable.patch index cb1f82a3..1d9cb434 100644 --- a/go-patches/0004-cmd-link-do-not-mark-resource-section-as-writable.patch +++ b/go-patches/0004-cmd-link-do-not-mark-resource-section-as-writable.patch @@ -1,7 +1,7 @@ From 8255e115f325a58fd4746741f9f35c2f54d70d63 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sun, 8 Nov 2020 11:11:27 +0100 -Subject: [PATCH 04/13] cmd/link: do not mark resource section as writable +Subject: [PATCH 04/14] cmd/link: do not mark resource section as writable Resources are immutable, and all other linkers set this section to be read-only and not read-write. Fix this oversight by rmoving the writable diff --git a/go-patches/0005-cmd-link-handle-grouped-resource-sections.patch b/go-patches/0005-cmd-link-handle-grouped-resource-sections.patch index 9e11d775..02af61a2 100644 --- a/go-patches/0005-cmd-link-handle-grouped-resource-sections.patch +++ b/go-patches/0005-cmd-link-handle-grouped-resource-sections.patch @@ -1,7 +1,7 @@ From 5d5d74f35dd3375cda8ef2ba8257547aad107ecb Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sun, 8 Nov 2020 11:57:42 +0100 -Subject: [PATCH 05/13] cmd/link: handle grouped resource sections +Subject: [PATCH 05/14] cmd/link: handle grouped resource sections The Go PE linker does not support enough generalized PE logic to properly handle .rsrc sections gracefully. Instead a few things are diff --git a/go-patches/0006-Revert-release-branch.go1.15-runtime-detect-services.patch b/go-patches/0006-Revert-release-branch.go1.15-runtime-detect-services.patch index 3cb0bdd2..cb21af9f 100644 --- a/go-patches/0006-Revert-release-branch.go1.15-runtime-detect-services.patch +++ b/go-patches/0006-Revert-release-branch.go1.15-runtime-detect-services.patch @@ -1,7 +1,7 @@ From e6b4c1b3beb4f843ed40abead27c8132d29a0db8 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Fri, 11 Sep 2020 13:04:11 +0200 -Subject: [PATCH 06/13] Revert "[release-branch.go1.15] runtime: detect +Subject: [PATCH 06/14] Revert "[release-branch.go1.15] runtime: detect services in signal handler" This reverts commit b1253d24e159129c778377c3a2a0bde15904a417. diff --git a/go-patches/0007-runtime-do-not-explicitly-exit-on-ctrl-handler.patch b/go-patches/0007-runtime-do-not-explicitly-exit-on-ctrl-handler.patch index 8479c1cc..4db19853 100644 --- a/go-patches/0007-runtime-do-not-explicitly-exit-on-ctrl-handler.patch +++ b/go-patches/0007-runtime-do-not-explicitly-exit-on-ctrl-handler.patch @@ -1,7 +1,7 @@ From af4eb34d920c0c727041ebf587e0de608068ed59 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Tue, 14 Jul 2020 01:41:03 -0600 -Subject: [PATCH 07/13] runtime: do not explicitly exit on ctrl handler +Subject: [PATCH 07/14] runtime: do not explicitly exit on ctrl handler The default ctrl+c handler should process exits in situations where it makes sense, like console apps, but not in situations where it doesn't, diff --git a/go-patches/0008-runtime-allow-callback-functions-with-up-to-8-argume.patch b/go-patches/0008-runtime-allow-callback-functions-with-up-to-8-argume.patch index 186610ae..0af97ce2 100644 --- a/go-patches/0008-runtime-allow-callback-functions-with-up-to-8-argume.patch +++ b/go-patches/0008-runtime-allow-callback-functions-with-up-to-8-argume.patch @@ -1,7 +1,7 @@ From 5a7c808cbacb8c0477395c5656c1eba3ef38cd6e Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Tue, 10 Nov 2020 21:42:36 +0100 -Subject: [PATCH 08/13] runtime: allow callback functions with up to 8 +Subject: [PATCH 08/14] runtime: allow callback functions with up to 8 arguments on windows/arm Previously, windows/arm programs would abort when trying to use diff --git a/go-patches/0009-runtime-use-CreateWaitableTimerEx-to-implement-uslee.patch b/go-patches/0009-runtime-use-CreateWaitableTimerEx-to-implement-uslee.patch index 90604341..83647c59 100644 --- a/go-patches/0009-runtime-use-CreateWaitableTimerEx-to-implement-uslee.patch +++ b/go-patches/0009-runtime-use-CreateWaitableTimerEx-to-implement-uslee.patch @@ -1,7 +1,7 @@ From 1a163f2e676e4664f2db3f614eb4168bda9d4fd8 Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Sun, 19 Jul 2020 16:06:48 +1000 -Subject: [PATCH 09/13] runtime: use CreateWaitableTimerEx to implement usleep +Subject: [PATCH 09/14] runtime: use CreateWaitableTimerEx to implement usleep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit diff --git a/go-patches/0010-runtime-allow-for-usleep2HighRes-to-run-without-TLS-.patch b/go-patches/0010-runtime-allow-for-usleep2HighRes-to-run-without-TLS-.patch index 3fe6935e..1c25f5b6 100644 --- a/go-patches/0010-runtime-allow-for-usleep2HighRes-to-run-without-TLS-.patch +++ b/go-patches/0010-runtime-allow-for-usleep2HighRes-to-run-without-TLS-.patch @@ -1,7 +1,7 @@ From 1edac4879e2d4a36d339353c151cb7b9871e135f Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Sat, 21 Nov 2020 14:56:26 +1100 -Subject: [PATCH 10/13] runtime: allow for usleep2HighRes to run without TLS +Subject: [PATCH 10/14] runtime: allow for usleep2HighRes to run without TLS setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 diff --git a/go-patches/0011-cmd-link-windows-arm-is-all-pie-so-mark-it-as-such.patch b/go-patches/0011-cmd-link-windows-arm-is-all-pie-so-mark-it-as-such.patch index 72cbb000..a2984678 100644 --- a/go-patches/0011-cmd-link-windows-arm-is-all-pie-so-mark-it-as-such.patch +++ b/go-patches/0011-cmd-link-windows-arm-is-all-pie-so-mark-it-as-such.patch @@ -1,7 +1,7 @@ From fee7906e1a7e62b655bea0f25c921572ee29fc44 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Thu, 26 Nov 2020 22:38:45 +0100 -Subject: [PATCH 11/13] cmd/link: windows/arm is all pie, so mark it as such +Subject: [PATCH 11/14] cmd/link: windows/arm is all pie, so mark it as such If the linker thinks that it's making an exe instead of a pie object, it won't apply relocations to the pclntab and we wind up with crashes like: diff --git a/go-patches/0012-runtime-adjust-address-calculation-in-identifying-ab.patch b/go-patches/0012-runtime-adjust-address-calculation-in-identifying-ab.patch index 2f3954db..38fcbb9b 100644 --- a/go-patches/0012-runtime-adjust-address-calculation-in-identifying-ab.patch +++ b/go-patches/0012-runtime-adjust-address-calculation-in-identifying-ab.patch @@ -1,7 +1,7 @@ From 8ec41fee1e6ef074ef74e56fe079c70c8a1d0548 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Fri, 27 Nov 2020 22:07:23 +0100 -Subject: [PATCH 12/13] runtime: adjust address calculation in identifying +Subject: [PATCH 12/14] runtime: adjust address calculation in identifying abort on windows/arm Apparently we're being called on arm 1 byte off, just like on 386 and diff --git a/go-patches/0013-cmd-compile-do-not-assume-TST-and-TEQ-set-V-on-arm.patch b/go-patches/0013-cmd-compile-do-not-assume-TST-and-TEQ-set-V-on-arm.patch index 296ca4a2..a8eb403d 100644 --- a/go-patches/0013-cmd-compile-do-not-assume-TST-and-TEQ-set-V-on-arm.patch +++ b/go-patches/0013-cmd-compile-do-not-assume-TST-and-TEQ-set-V-on-arm.patch @@ -1,7 +1,7 @@ From 58e426bb8175959d7bf32ea7c87e51f4706b309b Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 30 Nov 2020 10:41:46 +0100 -Subject: [PATCH 13/13] cmd/compile: do not assume TST and TEQ set V on arm +Subject: [PATCH 13/14] cmd/compile: do not assume TST and TEQ set V on arm These replacement rules assume that TST and TEQ set V. But TST and TEQ do not set V. This is a problem because instructions like LT are diff --git a/go-patches/0014-runtime-make-write-into-function-pointer.patch b/go-patches/0014-runtime-make-write-into-function-pointer.patch new file mode 100644 index 00000000..1618f02e --- /dev/null +++ b/go-patches/0014-runtime-make-write-into-function-pointer.patch @@ -0,0 +1,32 @@ +From d7b6f2bfcee5787d9055f5d7ec92c74e258e8fab Mon Sep 17 00:00:00 2001 +From: "Jason A. Donenfeld" +Date: Thu, 3 Dec 2020 13:29:58 +0100 +Subject: [PATCH 14/14] runtime: make write into function pointer + +Change-Id: I77a32ff7e1494324e8cc38e792e007f86d32672d +--- + src/runtime/time_nofake.go | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/src/runtime/time_nofake.go b/src/runtime/time_nofake.go +index 1912a94e87..0564448b2e 100644 +--- a/src/runtime/time_nofake.go ++++ b/src/runtime/time_nofake.go +@@ -23,9 +23,14 @@ func walltime() (sec int64, nsec int32) { + return walltime1() + } + ++var overrideWrite func(fd uintptr, p unsafe.Pointer, n int32) int32 ++ + // write must be nosplit on Windows (see write1) + // + //go:nosplit + func write(fd uintptr, p unsafe.Pointer, n int32) int32 { ++ if overrideWrite != nil { ++ return overrideWrite(fd, noescape(p), n) ++ } + return write1(fd, p, n) + } +-- +2.29.2 + diff --git a/manager/interfacecleanup.go b/manager/interfacecleanup.go index b6c8d6ae..58ed6615 100644 --- a/manager/interfacecleanup.go +++ b/manager/interfacecleanup.go @@ -18,8 +18,6 @@ import ( ) func cleanupStaleWintunInterfaces() { - defer printPanic() - m, err := mgr.Connect() if err != nil { return diff --git a/manager/ipc_server.go b/manager/ipc_server.go index 94d24e6f..19298016 100644 --- a/manager/ipc_server.go +++ b/manager/ipc_server.go @@ -467,7 +467,6 @@ func IPCServerListen(reader *os.File, writer *os.File, events *os.File, elevated } go func() { - defer printPanic() managerServicesLock.Lock() managerServices[service] = true managerServicesLock.Unlock() diff --git a/manager/service.go b/manager/service.go index 2afd796f..f439c186 100644 --- a/manager/service.go +++ b/manager/service.go @@ -7,12 +7,9 @@ package manager import ( "errors" - "fmt" "log" "os" "runtime" - "runtime/debug" - "strings" "sync" "syscall" "time" @@ -30,17 +27,6 @@ import ( type managerService struct{} -func printPanic() { - if x := recover(); x != nil { - for _, line := range append([]string{fmt.Sprint(x)}, strings.Split(string(debug.Stack()), "\n")...) { - if len(strings.TrimSpace(line)) > 0 { - log.Println(line) - } - } - panic(x) - } -} - func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest, changes chan<- svc.Status) (svcSpecificEC bool, exitCode uint32) { changes <- svc.Status{State: svc.StartPending} @@ -61,7 +47,6 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest serviceError = services.ErrorRingloggerOpen return } - defer printPanic() log.Println("Starting", version.UserAgent()) @@ -268,7 +253,6 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest goStartProcess := func(session uint32) { procsGroup.Add(1) go func() { - defer printPanic() startProcess(session) procsGroup.Done() }() diff --git a/manager/tunneltracker.go b/manager/tunneltracker.go index 6d376e4a..cfd28693 100644 --- a/manager/tunneltracker.go +++ b/manager/tunneltracker.go @@ -196,8 +196,6 @@ func trackService(service *mgr.Service, callback func(status uint32) bool) error } func trackTunnelService(tunnelName string, service *mgr.Service) { - defer printPanic() - defer func() { service.Close() log.Printf("[%s] Tunnel service tracker finished", tunnelName) diff --git a/manager/updatestate.go b/manager/updatestate.go index 9686e92f..e926e63d 100644 --- a/manager/updatestate.go +++ b/manager/updatestate.go @@ -24,8 +24,6 @@ const ( var updateState = UpdateStateUnknown func checkForUpdates() { - defer printPanic() - if !version.IsRunningOfficialVersion() { log.Println("Build is not official, so updates are disabled") updateState = UpdateStateUpdatesDisabledUnofficialBuild diff --git a/ringlogger/global.go b/ringlogger/global.go index 0d99e5a0..6f4458ca 100644 --- a/ringlogger/global.go +++ b/ringlogger/global.go @@ -8,6 +8,7 @@ package ringlogger import ( "log" "path/filepath" + "unsafe" "golang.zx2c4.com/wireguard/windows/conf" ) @@ -28,5 +29,39 @@ func InitGlobalLogger(tag string) error { } log.SetOutput(Global) log.SetFlags(0) + overrideWrite = globalWrite return nil } + +//go:linkname overrideWrite runtime.overrideWrite +var overrideWrite func(fd uintptr, p unsafe.Pointer, n int32) int32 + +var globalBuffer [maxLogLineLength - 1 - maxTagLength - 3]byte +var globalBufferLocation int + +//go:nosplit +func globalWrite(fd uintptr, p unsafe.Pointer, n int32) int32 { + b := (*[1 << 30]byte)(p)[:n] + for len(b) > 0 { + amountAvailable := len(globalBuffer) - globalBufferLocation + amountToCopy := len(b) + if amountToCopy > amountAvailable { + amountToCopy = amountAvailable + } + copy(globalBuffer[globalBufferLocation:], b[:amountToCopy]) + b = b[amountToCopy:] + globalBufferLocation += amountToCopy + foundNl := false + for i := globalBufferLocation - amountToCopy; i < globalBufferLocation; i++ { + if globalBuffer[i] == '\n' { + foundNl = true + break + } + } + if foundNl || len(b) > 0 { + Global.Write(globalBuffer[:globalBufferLocation]) + globalBufferLocation = 0 + } + } + return n +} diff --git a/ringlogger/ringlogger.go b/ringlogger/ringlogger.go index 84eb2fcc..fda4ddd9 100644 --- a/ringlogger/ringlogger.go +++ b/ringlogger/ringlogger.go @@ -21,6 +21,7 @@ import ( const ( maxLogLineLength = 512 + maxTagLength = 5 maxLines = 2048 magic = 0xbadbabe ) @@ -45,6 +46,9 @@ type Ringlogger struct { } func NewRinglogger(filename string, tag string) (*Ringlogger, error) { + if len(tag) > maxTagLength { + return nil, windows.ERROR_LABEL_TOO_LONG + } file, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE, 0600) if err != nil { return nil, err @@ -106,6 +110,11 @@ func (rl *Ringlogger) Write(p []byte) (n int, err error) { if rl.readOnly { return 0, io.ErrShortWrite } + ret := len(p) + p = bytes.TrimSpace(p) + if len(p) == 0 { + return ret, nil + } // Race: This isn't synchronized with the fetch_add below, so items might be slightly out of order. ts := time.Now().UnixNano() @@ -124,18 +133,20 @@ func (rl *Ringlogger) Write(p []byte) (n int, err error) { line.line[i] = 0 } - text := []byte(fmt.Sprintf("[%s] %s", rl.tag, bytes.TrimSpace(p))) - if len(text) > maxLogLineLength-1 { - text = text[:maxLogLineLength-1] + textLen := 3 + len(p) + len(rl.tag) + if textLen > maxLogLineLength-1 { + p = p[:maxLogLineLength-1-3-len(rl.tag)] } - line.line[len(text)] = 0 - copy(line.line[:], text[:]) + line.line[textLen] = 0 + line.line[0] = 0 // Null out the beginning and only let it extend after the other writes have completed + copy(line.line[1:], rl.tag) + line.line[1+len(rl.tag)] = ']' + line.line[2+len(rl.tag)] = ' ' + copy(line.line[3+len(rl.tag):], p[:]) + line.line[0] = '[' atomic.StoreInt64(&line.timeNs, ts) - windows.FlushViewOfFile((uintptr)(unsafe.Pointer(&rl.log.nextIndex)), unsafe.Sizeof(rl.log.nextIndex)) - windows.FlushViewOfFile((uintptr)(unsafe.Pointer(line)), unsafe.Sizeof(*line)) - - return len(p), nil + return ret, nil } func (rl *Ringlogger) WriteTo(out io.Writer) (n int64, err error) { -- cgit v1.2.3-59-g8ed1b