refactor(client): Refactor NetworkCallback to make the unit tests work https://github.com/open62541/open62541/commit/de9d691906547c9fc99e0a77198a80fe5bba54b1 fix(client): use after free in asyncServiceTimeoutCheck https://github.com/open62541/open62541/pull/5270 https://github.com/open62541/open62541/commit/713cdb419f0f844517afc549c7c1ef48d8498cfb Index: src/client/ua_client.c --- src/client/ua_client.c.orig +++ src/client/ua_client.c @@ -515,8 +515,17 @@ UA_Client_AsyncService_cancel(UA_Client *client, Async } void UA_Client_AsyncService_removeAll(UA_Client *client, UA_StatusCode statusCode) { + /* Make this function reentrant. One of the async callbacks could indirectly + * operate on the list. Moving all elements to a local list before iterating + * that. */ + UA_AsyncServiceList asyncServiceCalls = client->asyncServiceCalls; + LIST_INIT(&client->asyncServiceCalls); + if(asyncServiceCalls.lh_first) + asyncServiceCalls.lh_first->pointers.le_prev = &asyncServiceCalls.lh_first; + + /* Cancel and remove the elements from the local list */ AsyncServiceCall *ac, *ac_tmp; - LIST_FOREACH_SAFE(ac, &client->asyncServiceCalls, pointers, ac_tmp) { + LIST_FOREACH_SAFE(ac, &asyncServiceCalls, pointers, ac_tmp) { LIST_REMOVE(ac, pointers); UA_Client_AsyncService_cancel(client, ac, statusCode); UA_free(ac); @@ -627,16 +636,27 @@ UA_Client_removeCallback(UA_Client *client, UA_UInt64 static void asyncServiceTimeoutCheck(UA_Client *client) { + /* Make this function reentrant. One of the async callbacks could indirectly + * operate on the list. Moving all elements to a local list before iterating + * that. */ + UA_AsyncServiceList asyncServiceCalls; AsyncServiceCall *ac, *ac_tmp; UA_DateTime now = UA_DateTime_nowMonotonic(); + LIST_INIT(&asyncServiceCalls); LIST_FOREACH_SAFE(ac, &client->asyncServiceCalls, pointers, ac_tmp) { if(!ac->timeout) continue; if(ac->start + (UA_DateTime)(ac->timeout * UA_DATETIME_MSEC) <= now) { LIST_REMOVE(ac, pointers); - UA_Client_AsyncService_cancel(client, ac, UA_STATUSCODE_BADTIMEOUT); - UA_free(ac); + LIST_INSERT_HEAD(&asyncServiceCalls, ac, pointers); } + } + + /* Cancel and remove the elements from the local list */ + LIST_FOREACH_SAFE(ac, &asyncServiceCalls, pointers, ac_tmp) { + LIST_REMOVE(ac, pointers); + UA_Client_AsyncService_cancel(client, ac, UA_STATUSCODE_BADTIMEOUT); + UA_free(ac); } }