From 31c010080e7c0d643f8b19757202f8a50b1b3862 Mon Sep 17 00:00:00 2001
From: "Rob Swindell (on Debian Linux)" <rob@synchro.net>
Date: Wed, 22 Jan 2025 13:57:24 -0800
Subject: [PATCH] Add/use new function: listAddNodeWithFlags()
Fixes race condition/possible crash in listAddNodeData(), listAddNodeString()
and listAddNodeList(). These functions were modifying the node->flags *after*
a node was added to the list and list lock released.
This is most likely the cause of the issue caught by valgrind when running the
jsexec-testsuite:
https://gitlab.synchro.net/main/sbbs/-/jobs/498286
Thank you Deuce and valgrind, nice catch!
---
src/xpdev/link_list.c | 17 ++++++++++-------
src/xpdev/link_list.h | 3 +++
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/xpdev/link_list.c b/src/xpdev/link_list.c
index ab4ecf0dd7..9c44919932 100644
--- a/src/xpdev/link_list.c
+++ b/src/xpdev/link_list.c
@@ -569,7 +569,7 @@ static list_node_t* list_add_node(link_list_t* list, list_node_t* node, list_nod
return node;
}
-list_node_t* listAddNode(link_list_t* list, void* data, list_node_tag_t tag, list_node_t* after)
+list_node_t* listAddNodeWithFlags(link_list_t* list, void* data, list_node_tag_t tag, long flags, list_node_t* after)
{
list_node_t* node;
@@ -582,10 +582,16 @@ list_node_t* listAddNode(link_list_t* list, void* data, list_node_tag_t tag, lis
memset(node, 0, sizeof(list_node_t));
node->data = data;
node->tag = tag;
+ node->flags = flags;
return list_add_node(list, node, after);
}
+list_node_t* listAddNode(link_list_t* list, void* data, list_node_tag_t tag, list_node_t* after)
+{
+ return listAddNodeWithFlags(list, data, tag, 0, after);
+}
+
long listAddNodes(link_list_t* list, void** data, list_node_tag_t* tag, list_node_t* after)
{
long i;
@@ -610,11 +616,10 @@ list_node_t* listAddNodeData(link_list_t* list, const void* data, size_t length,
return NULL;
memcpy(buf, data, length);
- if ((node = listAddNode(list, buf, tag, after)) == NULL) {
+ if ((node = listAddNodeWithFlags(list, buf, tag, LINK_LIST_MALLOC, after)) == NULL) {
free(buf);
return NULL;
}
- node->flags |= LINK_LIST_MALLOC;
return node;
}
@@ -630,11 +635,10 @@ list_node_t* listAddNodeString(link_list_t* list, const char* str, list_node_tag
if ((buf = strdup(str)) == NULL)
return NULL;
- if ((node = listAddNode(list, buf, tag, after)) == NULL) {
+ if ((node = listAddNodeWithFlags(list, buf, tag, LINK_LIST_MALLOC, after)) == NULL) {
free(buf);
return NULL;
}
- node->flags |= LINK_LIST_MALLOC;
return node;
}
@@ -668,9 +672,8 @@ long listAddNodeList(link_list_t* list, const link_list_t* src, list_node_t* aft
return -1;
for (src_node = src->first; src_node != NULL; src_node = src_node->next, count++) {
- if ((node = listAddNode(list, src_node->data, src_node->tag, node == NULL ? after:node)) == NULL)
+ if ((node = listAddNodeWithFlags(list, src_node->data, src_node->tag, src_node->flags, node == NULL ? after:node)) == NULL)
return count;
- node->flags = src_node->flags;
}
return count;
diff --git a/src/xpdev/link_list.h b/src/xpdev/link_list.h
index 61d2f3c304..3a12ffc0f5 100644
--- a/src/xpdev/link_list.h
+++ b/src/xpdev/link_list.h
@@ -148,6 +148,9 @@ DLLEXPORT bool listNodeIsLocked(const list_node_t*);
/* Add node to list, returns pointer to new node or NULL on error */
DLLEXPORT list_node_t* listAddNode(link_list_t*, void* data, list_node_tag_t, list_node_t * after /* NULL=insert */);
+/* Add node to list with flags, returns pointer to new node or NULL on error */
+DLLEXPORT list_node_t* listAddNodeWithFlags(link_list_t*, void* data, list_node_tag_t, long flags, list_node_t * after /* NULL=insert */);
+
/* Add array of node data to list, returns number of nodes added (or negative on error) */
/* tag array may be NULL */
DLLEXPORT long listAddNodes(link_list_t*, void** data, list_node_tag_t*, list_node_t* after /* NULL=insert */);
--
GitLab