commit 83420dc0fca97afc271417c184306da1643e5280
author Travis Geiselbrecht <travisg@google.com> Wed Sep 19 14:23:20 2018 -0700
committer Travis Geiselbrecht <travisg@google.com> Fri Sep 21 18:40:40 2018 +0000
tree eaebfea3db03095aec88befa904d4552cf2f2bdd
parent 727ba628960c53a719307d9f9ae226e85992d255

[kernel][lib][cbuf] fix potential deadlock

Signalling the cbuf signal inside the cbuf's spinlock is potentially bad
since it can reschedule while holding the lock. Move the event signal
outside of the lock.

This isn't happening now since all the callers of this routine are
currently inside an interrupt which implicitly disables local
rescheduling.

Change-Id: I3aca2963bfd7252817aa90d8dd60931ebb2a328e
Tested: compile and run, validating that serial IO continues to work.

kernel/lib/cbuf/cbuf.cpp

diff --git a/kernel/lib/cbuf/cbuf.cpp b/kernel/lib/cbuf/cbuf.cpp
index 44c8cd7..3aac638 100644
--- a/kernel/lib/cbuf/cbuf.cpp
+++ b/kernel/lib/cbuf/cbuf.cpp
@@ -51,20 +51,22 @@
 size_t cbuf_write_char(cbuf_t* cbuf, char c) {
     DEBUG_ASSERT(cbuf);
 
-    AutoSpinLock guard(&cbuf->lock);
-
     size_t ret = 0;
-    if (cbuf_space_avail(cbuf) > 0) {
-        cbuf->buf[cbuf->head] = c;
+    {
+        AutoSpinLock guard(&cbuf->lock);
 
-        cbuf->head = inc_pointer(cbuf, cbuf->head, 1);
-        ret = 1;
+        if (cbuf_space_avail(cbuf) > 0) {
+            cbuf->buf[cbuf->head] = c;
 
-        if (cbuf->head != cbuf->tail) {
-            event_signal(&cbuf->event, true);
+            cbuf->head = inc_pointer(cbuf, cbuf->head, 1);
+            ret = 1;
         }
     }
 
+    if (ret > 0) {
+        event_signal(&cbuf->event, true);
+    }
+
     return ret;
 }