Fix clobber of current in kthread.c
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 3 Oct 2016 19:03:50 +0000 (15:03 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 6 Oct 2016 19:41:48 +0000 (15:41 -0400)
Originally, there wasn't a KTH_SAVE_ADDR_SPACE flag.  When I added that, I
didn't update this code.  The resulting bug was that if we had to undo a
kthread swap, that kthread was for a ktask (which doesn't have a proc), and
we had a process's address space loaded, then we'd clobber current
(clearing it).  That would result in a reference counting problem, since we
effectively deleted a counted reference to whatever process was current.
I'd see this on occasion under heavy networking and process load.

This also clears kthread->proc whenever the kthread is not blocked.
Previously, we were leaving the value of the uncounted proc reference.  The
code was okay, but it was surprising when debugging and was a source for
potential bugs.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/src/kthread.c

index 76febba..4ec3723 100644 (file)
@@ -106,6 +106,7 @@ void restart_kthread(struct kthread *kthread)
                        proc_decref(pcpui->cur_proc);
                /* We also transfer our counted ref from kthread->proc to cur_proc */
                pcpui->cur_proc = kthread->proc;
+               kthread->proc = 0;
        }
        /* Finally, restart our thread */
        longjmp(&kthread->context, 1);
@@ -411,7 +412,7 @@ void sem_down(struct semaphore *sem)
                 * clear current and transfer the refcnt to kthread->proc. */
                proc_incref(kthread->proc, 1);
        } else {
-               kthread->proc = 0;
+               assert(kthread->proc == 0);
        }
        if (setjmp(&kthread->context))
                goto block_return_path;
@@ -440,9 +441,10 @@ void sem_down(struct semaphore *sem)
        debug_unlock_semlist();
        printd("[kernel] Didn't sleep, unwinding...\n");
        /* Restore the core's current and default stacktop */
-       current = kthread->proc;                        /* arguably unnecessary */
-       if (kthread->proc)
+       if (kthread->flags & KTH_SAVE_ADDR_SPACE) {
                proc_decref(kthread->proc);
+               kthread->proc = 0;
+       }
        set_stack_top(kthread->stacktop);
        pcpui->cur_kthread = kthread;
        /* Save the allocs as the spare */