Add more recover() at goroutine start points

This is a double-edged sword though as blind panic recovery
can lead to consistency issues in program state.

In particular, halting imapsql update push due to panic can lead
to a deadlock in IMAP code.

Panic in MTA-STS cache maintenance routine can lead to degraded
security.
This commit is contained in:
fox.cpp 2020-09-10 20:41:12 +03:00
parent f9d5c0cb02
commit ec02cca6f8
No known key found for this signature in database
GPG key ID: 5B991F6215D2FCC0
3 changed files with 32 additions and 1 deletions

View file

@ -20,6 +20,7 @@ package msgpipeline
import (
"context"
"runtime/debug"
"sync"
"github.com/emersion/go-message/textproto"
@ -177,6 +178,14 @@ func (cr *checkRunner) runAndMergeResults(states []module.CheckState, runner fun
state := state
data.wg.Add(1)
go func() {
defer func() {
data.wg.Done()
if err := recover(); err != nil {
stack := debug.Stack()
log.Printf("panic during check execution: %v\n%s", err, stack)
}
}()
subCheckRes := runner(state)
// We check the length because we don't want to take locks
@ -213,7 +222,6 @@ func (cr *checkRunner) runAndMergeResults(states []module.CheckState, runner fun
cr.log.Error("no check action", subCheckRes.Reason)
}
data.wg.Done()
}()
}

View file

@ -33,6 +33,7 @@ import (
"fmt"
"os"
"path/filepath"
"runtime/debug"
"runtime/trace"
"strconv"
"strings"
@ -387,6 +388,11 @@ func (store *Storage) EnableUpdatePipe(mode updatepipe.BackendMode) error {
defer func() {
store.updPushStop <- struct{}{}
close(wrapped)
if err := recover(); err != nil {
stack := debug.Stack()
log.Printf("panic during imapsql update push: %v\n%s", err, stack)
}
}()
for {

View file

@ -22,6 +22,7 @@ import (
"context"
"crypto/tls"
"os"
"runtime/debug"
"time"
"github.com/foxcpp/go-mtasts"
@ -107,6 +108,15 @@ func (c *mtastsPolicy) StartUpdater() {
}
func (c *mtastsPolicy) updater() {
defer func() {
if err := recover(); err != nil {
stack := debug.Stack()
log.Printf("panic during MTA-STS update: %v\n%s", err, stack)
log.Printf("MTA-STS cache refresh disabled due to critical error")
c.updaterStop = nil
}
}()
// Always update cache on start-up since we may have been down for some
// time.
c.log.Debugln("updating MTA-STS cache...")
@ -395,6 +405,13 @@ func (c *daneDelivery) PrepareConn(ctx context.Context, mx string) {
c.tlsaFut = future.New()
go func() {
defer func() {
if err := recover(); err != nil {
stack := debug.Stack()
log.Printf("panic during extended resolver lookup: %v\n%s", err, stack)
}
}()
ad, recs, err := c.c.extResolver.AuthLookupTLSA(ctx, "25", "tcp", mx)
if err != nil {
c.tlsaFut.Set([]dns.TLSA{}, err)