From ebbe57d0ab38ab2f27b75a83efa8459dc88a2612 Mon Sep 17 00:00:00 2001 From: ardevd Date: Tue, 3 Jun 2025 07:39:21 +0200 Subject: [PATCH] fix(syncthing): avoid writing panic log to nil fd (#10154) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Purpose This change fixes a logical bug in the panic log writing where we could end up writing to a uninitialized file descriptor. On the very first iteration, `panicFd` is nil. We enter the if `panicFd == nil { … }` block, check for “panic:” or “fatal error:”, and if neither matches, we skip instantiating `panicFd` altogether. However, immediately after, still within `if panicFd == nil { … }`, we call `panicFd.WriteString("Panic at ...")`. But `panicFd` would in this case be `nil`, which will cause a run‐time panic. It's not clear to me why panicFd is only initialized if the lines start with "panic:" or "fatal error:" so I've left that logic untouched. With this change we at least avoid the risk of writing to a nil filedescriptor. ## Authorship Your name and email will be added automatically to the AUTHORS file based on the commit metadata. --------- Co-authored-by: Jakob Borg --- cmd/syncthing/monitor.go | 48 +++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/cmd/syncthing/monitor.go b/cmd/syncthing/monitor.go index f91ee8323..932ae1309 100644 --- a/cmd/syncthing/monitor.go +++ b/cmd/syncthing/monitor.go @@ -238,19 +238,18 @@ func copyStderr(stderr io.Reader, dst io.Writer) { return } - if panicFd == nil { - dst.Write([]byte(line)) + dst.Write([]byte(line)) - if strings.HasPrefix(line, "panic:") || strings.HasPrefix(line, "fatal error:") { - panicFd, err = os.Create(locations.GetTimestamped(locations.PanicLog)) - if err != nil { - l.Warnln("Create panic log:", err) - continue - } + if panicFd == nil && (strings.HasPrefix(line, "panic:") || strings.HasPrefix(line, "fatal error:")) { + panicFd, err = os.Create(locations.GetTimestamped(locations.PanicLog)) + if err != nil { + l.Warnln("Create panic log:", err) + continue + } - l.Warnf("Panic detected, writing to \"%s\"", panicFd.Name()) - if strings.Contains(line, "leveldb") && strings.Contains(line, "corrupt") { - l.Warnln(` + l.Warnf("Panic detected, writing to \"%s\"", panicFd.Name()) + if strings.Contains(line, "leveldb") && strings.Contains(line, "corrupt") { + l.Warnln(` ********************************************************************************* * Crash due to corrupt database. * * * @@ -263,22 +262,21 @@ func copyStderr(stderr io.Reader, dst io.Writer) { * https://docs.syncthing.net/users/faq.html#my-syncthing-database-is-corrupt * ********************************************************************************* `) - } else { - l.Warnln("Please check for existing issues with similar panic message at https://github.com/syncthing/syncthing/issues/") - l.Warnln("If no issue with similar panic message exists, please create a new issue with the panic log attached") - } - - stdoutMut.Lock() - for _, line := range stdoutFirstLines { - panicFd.WriteString(line) - } - panicFd.WriteString("...\n") - for _, line := range stdoutLastLines { - panicFd.WriteString(line) - } - stdoutMut.Unlock() + } else { + l.Warnln("Please check for existing issues with similar panic message at https://github.com/syncthing/syncthing/issues/") + l.Warnln("If no issue with similar panic message exists, please create a new issue with the panic log attached") } + stdoutMut.Lock() + for _, line := range stdoutFirstLines { + panicFd.WriteString(line) + } + panicFd.WriteString("...\n") + for _, line := range stdoutLastLines { + panicFd.WriteString(line) + } + stdoutMut.Unlock() + panicFd.WriteString("Panic at " + time.Now().Format(time.RFC3339) + "\n") }