Bladeren bron

Bug in processRepeatedJobs()
invalid pointer
UA_Server_cleanup may destroy list of repeatedJobs

Jörg Schüler-Maroldt 9 jaren geleden
bovenliggende
commit
a5a8d0fac3
4 gewijzigde bestanden met toevoegingen van 21 en 2 verwijderingen
  1. 3 0
      include/ua_server.h
  2. 5 1
      include/ua_types.h
  3. 1 1
      src/server/ua_server.c
  4. 12 0
      src/server/ua_server_worker.c

+ 3 - 0
include/ua_server.h

@@ -619,6 +619,9 @@ UA_Server_addInstanceOf(UA_Server *server, UA_NodeId nodeId, const UA_QualifiedN
                         const UA_ExpandedNodeId typeDefinition, UA_InstantiationCallback callback, void *handle, 
                         UA_NodeId *createdNodeId);
 
+// Debug Stuff
+void UA_Server_cleanup(UA_Server *server, void *nothing);
+
 #ifdef __cplusplus
 } // extern "C"
 #endif

+ 5 - 1
include/ua_types.h

@@ -350,7 +350,11 @@ UA_DateTimeStruct UA_EXPORT UA_DateTime_toStruct(UA_DateTime time);
 
 /* Guid */
 UA_Guid UA_EXPORT * UA_Guid_new(void);
-static UA_INLINE void UA_Guid_init(UA_Guid *p) { *p = (UA_Guid){0,0,0,{0,0,0,0,0,0,0,0}}; }
+static UA_INLINE void UA_Guid_init(UA_Guid *p) 
+{ 
+    //*p = (UA_Guid){0,0,0,{0,0,0,0,0,0,0,0}}; 
+    memset(p, 0, sizeof(*p));
+}
 void UA_EXPORT UA_Guid_delete(UA_Guid *p);
 static UA_INLINE void UA_Guid_deleteMembers(UA_Guid *p) { }
 static UA_INLINE UA_StatusCode UA_Guid_copy(const UA_Guid *src, UA_Guid *dst) { *dst = *src; return UA_STATUSCODE_GOOD; }

+ 1 - 1
src/server/ua_server.c

@@ -184,7 +184,7 @@ void UA_Server_delete(UA_Server *server) {
 }
 
 /* Recurring cleanup. Removing unused and timed-out channels and sessions */
-static void UA_Server_cleanup(UA_Server *server, void *nothing) {
+/*static */void UA_Server_cleanup(UA_Server *server, void *nothing) {
     UA_DateTime now = UA_DateTime_now();
     UA_SessionManager_cleanupTimedOut(&server->sessionManager, server, now);
     UA_SecureChannelManager_cleanupTimedOut(&server->secureChannelManager, now);

+ 12 - 0
src/server/ua_server_worker.c

@@ -346,7 +346,17 @@ static UA_UInt16 processRepeatedJobs(UA_Server *server) {
         dispatchJobs(server, jobsCopy, tw->jobsSize); // frees the job pointer
 #else
         for(size_t i=0;i<tw->jobsSize;i++)
+        {
             processJobs(server, &tw->jobs[i].job, 1); // does not free the job ptr
+            if (tw->jobs[i].job.type == UA_JOBTYPE_METHODCALL
+                && tw->jobs[i].job.job.methodCall.method == UA_Server_cleanup)
+            {
+                // UA_Server_cleanup may delete members
+                // next is invalid Pointer
+                // this no solution
+                next = LIST_NEXT(tw, pointers);
+            }
+        }
 #endif
         tw->nextTime += tw->interval;
         struct RepeatedJobs *prevTw = tw; // after which tw do we insert?
@@ -357,6 +367,8 @@ static UA_UInt16 processRepeatedJobs(UA_Server *server) {
             prevTw = n;
         }
         if(prevTw != tw) {
+            // this is very dangerous
+            // so UA_Server_cleanup job comes up
             LIST_REMOVE(tw, pointers);
             LIST_INSERT_AFTER(prevTw, tw, pointers);
         }