From 03ca20d3e470e0d462abe1ed8d222d267ecc9ab7 Mon Sep 17 00:00:00 2001
From: Chris Spencer <spencercw@gmail.com>
Date: Tue, 11 Feb 2025 16:08:20 +0000
Subject: [PATCH] [client] overlay/msg: fix race condition in render

If an overlay is closed with overlayMsg_close, the message can be freed
while it is still being used by msg_render, resulting in a segfault. Lock
the message list for the duration of msg_render to fix this.
---
 client/src/overlay/msg.c   | 10 ++++++++--
 common/include/common/ll.h | 13 +++++++-----
 common/src/ll.c            | 41 +++++++++++++++++++++-----------------
 3 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/client/src/overlay/msg.c b/client/src/overlay/msg.c
index 25cc757e..6694d096 100644
--- a/client/src/overlay/msg.c
+++ b/client/src/overlay/msg.c
@@ -85,9 +85,14 @@ static bool msg_needsOverlay(void * udata)
 static int msg_render(void * udata, bool interactive, struct Rect * windowRects,
     int maxRects)
 {
+  ll_lock(l_msg.messages);
+
   struct Msg * msg;
-  if (!ll_peek_head(l_msg.messages, (void **)&msg))
+  if (!ll_peek_head_nl(l_msg.messages, (void **)&msg))
+  {
+    ll_unlock(l_msg.messages);
     return 0;
+  }
 
   ImVec2 * screen = overlayGetScreenSize();
   igSetNextWindowBgAlpha(0.8f);
@@ -163,7 +168,7 @@ static int msg_render(void * udata, bool interactive, struct Rect * windowRects,
 
   if (destroy)
   {
-    (void)ll_shift(l_msg.messages, NULL);
+    (void)ll_shift_nl(l_msg.messages, NULL);
     freeMsg(msg);
     app_invalidateOverlay(false);
   }
@@ -171,6 +176,7 @@ static int msg_render(void * udata, bool interactive, struct Rect * windowRects,
   overlayGetImGuiRect(windowRects);
   igEnd();
 
+  ll_unlock(l_msg.messages);
   return 1;
 }
 
diff --git a/common/include/common/ll.h b/common/include/common/ll.h
index 4ab68071..b614e000 100644
--- a/common/include/common/ll.h
+++ b/common/include/common/ll.h
@@ -41,11 +41,14 @@ struct ll
 };
 
 struct ll *  ll_new(void);
-void         ll_free     (struct ll * list);
-void         ll_push     (struct ll * list, void * data);
-bool         ll_shift    (struct ll * list, void ** data);
-bool         ll_peek_head(struct ll * list, void ** data);
-bool         ll_peek_tail(struct ll * list, void ** data);
+void         ll_free        (struct ll * list);
+void         ll_push        (struct ll * list, void * data);
+bool         ll_shift       (struct ll * list, void ** data);
+bool         ll_shift_nl    (struct ll * list, void ** data);
+bool         ll_peek_head   (struct ll * list, void ** data);
+bool         ll_peek_head_nl(struct ll * list, void ** data);
+bool         ll_peek_tail   (struct ll * list, void ** data);
+bool         ll_peek_tail_nl(struct ll * list, void ** data);
 
 #define ll_lock(ll) LG_LOCK((ll)->lock)
 #define ll_unlock(ll) LG_UNLOCK((ll)->lock)
diff --git a/common/src/ll.c b/common/src/ll.c
index cbbc3455..0037d1af 100644
--- a/common/src/ll.c
+++ b/common/src/ll.c
@@ -83,15 +83,18 @@ void ll_push(struct ll * list, void * data)
 bool ll_shift(struct ll * list, void ** data)
 {
   LG_LOCK(list->lock);
+  bool result = ll_shift_nl(list, data);
+  LG_UNLOCK(list->lock);
+  return result;
+}
+
+bool ll_shift_nl(struct ll * list, void ** data)
+{
   if (!list->head)
-  {
-    LG_UNLOCK(list->lock);
     return false;
-  }
 
   struct ll_item * item = list->head;
   ll_removeNL(list, item);
-  LG_UNLOCK(list->lock);
 
   if (data)
     *data = item->data;
@@ -103,29 +106,31 @@ bool ll_shift(struct ll * list, void ** data)
 bool ll_peek_head(struct ll * list, void ** data)
 {
   LG_LOCK(list->lock);
-  if (!list->head)
-  {
-    LG_UNLOCK(list->lock);
-    return false;
-  }
-
-  *data = list->head->data;
+  bool result = ll_peek_head_nl(list, data);
   LG_UNLOCK(list->lock);
+  return result;
+}
 
+bool ll_peek_head_nl(struct ll * list, void ** data)
+{
+  if (!list->head)
+    return false;
+  *data = list->head->data;
   return true;
 }
 
 bool ll_peek_tail(struct ll * list, void ** data)
 {
   LG_LOCK(list->lock);
-  if (!list->tail)
-  {
-    LG_UNLOCK(list->lock);
-    return false;
-  }
-
-  *data = list->tail->data;
+  bool result = ll_peek_tail_nl(list, data);
   LG_UNLOCK(list->lock);
+  return result;
+}
 
+bool ll_peek_tail_nl(struct ll * list, void ** data)
+{
+  if (!list->tail)
+    return false;
+  *data = list->tail->data;
   return true;
 }