[release-branch.go1.1] runtime: fix crash in select

««« CL 9311043 / 53bc96b4c0c7
runtime: fix crash in select
runtime.park() can access freed select descriptor
due to a racing free in another thread.
See the comment for details.

Slightly modified version of dvyukov's CL 9259045.

No test yet.  Before this CL, the test described in issue 5422
would fail about every 40 times for me.  With this CL, I ran
the test 5900 times with no failures.

Fixes #5422.

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/9311043
»»»

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/9304044

diff --git a/src/pkg/runtime/chan.c b/src/pkg/runtime/chan.c
index 69b90bda5699ab64844ff2b03f159a6ff2eddf03..fba36a4c345801c2b4663dceb25b48d20eb043d9 100644 (file)
--- a/src/pkg/runtime/chan.c
+++ b/src/pkg/runtime/chan.c
@@ -809,16 +809,27 @@ sellock(Select *sel)
 static void
 selunlock(Select *sel)
 {
-       uint32 i;
-       Hchan *c, *c0;
+       int32 i, n, r;
+       Hchan *c;
 
-       c = nil;
-       for(i=sel->ncase; i-->0;) {
-               c0 = sel->lockorder[i];
-               if(c0 && c0 != c) {
-                       c = c0;
-                       runtime·unlock(c);
-               }
+       // We must be very careful here to not touch sel after we have unlocked
+       // the last lock, because sel can be freed right after the last unlock.
+       // Consider the following situation.
+       // First M calls runtime·park() in runtime·selectgo() passing the sel.
+       // Once runtime·park() has unlocked the last lock, another M makes
+       // the G that calls select runnable again and schedules it for execution.
+       // When the G runs on another M, it locks all the locks and frees sel.
+       // Now if the first M touches sel, it will access freed memory.
+       n = (int32)sel->ncase;
+       r = 0;
+       // skip the default case
+       if(n>0 && sel->lockorder[0] == nil)
+               r = 1;
+       for(i = n-1; i >= r; i--) {
+               c = sel->lockorder[i];
+               if(i>0 && sel->lockorder[i-1] == c)
+                       continue;  // will unlock it on the next iteration
+               runtime·unlock(c);
        }
 }