Commit f946a7ca authored by Dmitriy Vyukov's avatar Dmitriy Vyukov

runtime: fix memory corruption and leak in recursive panic handling

Recursive panics leave dangling Panic structs in g->panic stack.
At best it leads to a Defer leak and incorrect output on a subsequent panic.
At worst it arbitrary corrupts heap.

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/72480043
parent b08156cd
...@@ -132,6 +132,18 @@ func TestThreadExhaustion(t *testing.T) { ...@@ -132,6 +132,18 @@ func TestThreadExhaustion(t *testing.T) {
} }
} }
func TestRecursivePanic(t *testing.T) {
output := executeTest(t, recursivePanicSource, nil)
want := `wrap: bad
panic: again
`
if !strings.HasPrefix(output, want) {
t.Fatalf("output does not start with %q:\n%s", want, output)
}
}
const crashSource = ` const crashSource = `
package main package main
...@@ -272,3 +284,29 @@ func main() { ...@@ -272,3 +284,29 @@ func main() {
} }
} }
` `
const recursivePanicSource = `
package main
import (
"fmt"
)
func main() {
func() {
defer func() {
fmt.Println(recover())
}()
var x [8192]byte
func(x [8192]byte) {
defer func() {
if err := recover(); err != nil {
panic("wrap: " + err.(string))
}
}()
panic("bad")
}(x)
}()
panic("again")
}
`
...@@ -205,12 +205,14 @@ printpanics(Panic *p) ...@@ -205,12 +205,14 @@ printpanics(Panic *p)
} }
static void recovery(G*); static void recovery(G*);
static void abortpanic(Panic*);
static FuncVal abortpanicV = { (void(*)(void))abortpanic };
// The implementation of the predeclared function panic. // The implementation of the predeclared function panic.
void void
runtime·panic(Eface e) runtime·panic(Eface e)
{ {
Defer *d; Defer *d, dabort;
Panic p; Panic p;
void *pc, *argp; void *pc, *argp;
...@@ -220,6 +222,12 @@ runtime·panic(Eface e) ...@@ -220,6 +222,12 @@ runtime·panic(Eface e)
p.stackbase = g->stackbase; p.stackbase = g->stackbase;
g->panic = &p; g->panic = &p;
dabort.fn = &abortpanicV;
dabort.siz = sizeof(&p);
dabort.args[0] = &p;
dabort.argp = (void*)-1; // unused because abortpanic never recovers
dabort.special = true;
for(;;) { for(;;) {
d = g->defer; d = g->defer;
if(d == nil) if(d == nil)
...@@ -229,10 +237,31 @@ runtime·panic(Eface e) ...@@ -229,10 +237,31 @@ runtime·panic(Eface e)
g->ispanic = true; // rock for runtime·newstack, where runtime·newstackcall ends up g->ispanic = true; // rock for runtime·newstack, where runtime·newstackcall ends up
argp = d->argp; argp = d->argp;
pc = d->pc; pc = d->pc;
// The deferred function may cause another panic,
// so newstackcall may not return. Set up a defer
// to mark this panic aborted if that happens.
dabort.link = g->defer;
g->defer = &dabort;
p.defer = d;
runtime·newstackcall(d->fn, (byte*)d->args, d->siz); runtime·newstackcall(d->fn, (byte*)d->args, d->siz);
// Newstackcall did not panic. Remove dabort.
if(g->defer != &dabort)
runtime·throw("bad defer entry in panic");
g->defer = dabort.link;
freedefer(d); freedefer(d);
if(p.recovered) { if(p.recovered) {
g->panic = p.link; g->panic = p.link;
// Aborted panics are marked but remain on the g->panic list.
// Recovery will unwind the stack frames containing their Panic structs.
// Remove them from the list and free the associated defers.
while(g->panic && g->panic->aborted) {
freedefer(g->panic->defer);
g->panic = g->panic->link;
}
if(g->panic == nil) // must be done with signal if(g->panic == nil) // must be done with signal
g->sig = 0; g->sig = 0;
// Pass information about recovering frame to recovery. // Pass information about recovering frame to recovery.
...@@ -250,6 +279,12 @@ runtime·panic(Eface e) ...@@ -250,6 +279,12 @@ runtime·panic(Eface e)
runtime·exit(1); // not reached runtime·exit(1); // not reached
} }
static void
abortpanic(Panic *p)
{
p->aborted = true;
}
// Unwind the stack after a deferred function calls recover // Unwind the stack after a deferred function calls recover
// after a panic. Then arrange to continue running as though // after a panic. Then arrange to continue running as though
// the caller of the deferred function returned normally. // the caller of the deferred function returned normally.
......
...@@ -733,7 +733,9 @@ struct Panic ...@@ -733,7 +733,9 @@ struct Panic
Eface arg; // argument to panic Eface arg; // argument to panic
uintptr stackbase; // g->stackbase in panic uintptr stackbase; // g->stackbase in panic
Panic* link; // link to earlier panic Panic* link; // link to earlier panic
Defer* defer; // current executing defer
bool recovered; // whether this panic is over bool recovered; // whether this panic is over
bool aborted; // the panic was aborted
}; };
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment