src/emc: cppcheck warns about null pointer derefences upon memory allocation failures

[smoe]

I do not know how to react in such situations. Does the machine stop when the controller crashes? This may be worthwhile to state in the documentation if this is the case. In former times some aberrant java script in a browser was sufficient to drain all memory, so I do not find the scenario completely absurd to expect.

Checking src/emc/usr_intf/schedrmt.cc ...
src/emc/usr_intf/schedrmt.cc:1157:3: warning: If memory allocation fails, then there is a possible null pointer dereference: context [nullPointerOutOfMemory]
  context->cliSock = client_sockfd;
  ^
src/emc/usr_intf/schedrmt.cc:1156:41: note: Assuming allocation function fails
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
                                        ^
src/emc/usr_intf/schedrmt.cc:1156:13: note: Assignment 'context=(struct connectionRecType*)std::malloc(sizeof(struct connectionRecType))', assigned value is 0
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
            ^
src/emc/usr_intf/schedrmt.cc:1157:3: note: Null pointer dereference
  context->cliSock = client_sockfd;
  ^
src/emc/usr_intf/schedrmt.cc:1158:3: warning: If memory allocation fails, then there is a possible null pointer dereference: context [nullPointerOutOfMemory]
  context->linked = false;
  ^
src/emc/usr_intf/schedrmt.cc:1156:41: note: Assuming allocation function fails
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
                                        ^
src/emc/usr_intf/schedrmt.cc:1156:13: note: Assignment 'context=(struct connectionRecType*)std::malloc(sizeof(struct connectionRecType))', assigned value is 0
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
            ^
src/emc/usr_intf/schedrmt.cc:1158:3: note: Null pointer dereference
  context->linked = false;
  ^
src/emc/usr_intf/schedrmt.cc:1159:3: warning: If memory allocation fails, then there is a possible null pointer dereference: context [nullPointerOutOfMemory]
  context->echo = true;
  ^
src/emc/usr_intf/schedrmt.cc:1156:41: note: Assuming allocation function fails
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
                                        ^
src/emc/usr_intf/schedrmt.cc:1156:13: note: Assignment 'context=(struct connectionRecType*)std::malloc(sizeof(struct connectionRecType))', assigned value is 0
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
            ^
src/emc/usr_intf/schedrmt.cc:1159:3: note: Null pointer dereference
  context->echo = true;
  ^
src/emc/usr_intf/schedrmt.cc:1160:3: warning: If memory allocation fails, then there is a possible null pointer dereference: context [nullPointerOutOfMemory]
  context->verbose = false;
  ^
src/emc/usr_intf/schedrmt.cc:1156:41: note: Assuming allocation function fails
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
                                        ^
src/emc/usr_intf/schedrmt.cc:1156:13: note: Assignment 'context=(struct connectionRecType*)std::malloc(sizeof(struct connectionRecType))', assigned value is 0
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
            ^
src/emc/usr_intf/schedrmt.cc:1160:3: note: Null pointer dereference
  context->verbose = false;
  ^
src/emc/usr_intf/schedrmt.cc:1161:17: warning: If memory allocation fails, then there is a possible null pointer dereference: context [nullPointerOutOfMemory]
  rtapi_strxcpy(context->version, "1.0");
                ^
src/emc/usr_intf/schedrmt.cc:1156:41: note: Assuming allocation function fails
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
                                        ^
src/emc/usr_intf/schedrmt.cc:1156:13: note: Assignment 'context=(struct connectionRecType*)std::malloc(sizeof(struct connectionRecType))', assigned value is 0
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
            ^
src/emc/usr_intf/schedrmt.cc:1161:17: note: Null pointer dereference
  rtapi_strxcpy(context->version, "1.0");
                ^
src/emc/usr_intf/schedrmt.cc:1162:17: warning: If memory allocation fails, then there is a possible null pointer dereference: context [nullPointerOutOfMemory]
  rtapi_strxcpy(context->hostName, "Default");
                ^
src/emc/usr_intf/schedrmt.cc:1156:41: note: Assuming allocation function fails
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
                                        ^
src/emc/usr_intf/schedrmt.cc:1156:13: note: Assignment 'context=(struct connectionRecType*)std::malloc(sizeof(struct connectionRecType))', assigned value is 0
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
            ^
src/emc/usr_intf/schedrmt.cc:1162:17: note: Null pointer dereference
  rtapi_strxcpy(context->hostName, "Default");
                ^
src/emc/usr_intf/schedrmt.cc:1163:3: warning: If memory allocation fails, then there is a possible null pointer dereference: context [nullPointerOutOfMemory]
  context->enabled = false;
  ^
src/emc/usr_intf/schedrmt.cc:1156:41: note: Assuming allocation function fails
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
                                        ^
src/emc/usr_intf/schedrmt.cc:1156:13: note: Assignment 'context=(struct connectionRecType*)std::malloc(sizeof(struct connectionRecType))', assigned value is 0
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
            ^
src/emc/usr_intf/schedrmt.cc:1163:3: note: Null pointer dereference
  context->enabled = false;
  ^
src/emc/usr_intf/schedrmt.cc:1164:3: warning: If memory allocation fails, then there is a possible null pointer dereference: context [nullPointerOutOfMemory]
  context->commMode = 0;
  ^
src/emc/usr_intf/schedrmt.cc:1156:41: note: Assuming allocation function fails
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
                                        ^
src/emc/usr_intf/schedrmt.cc:1156:13: note: Assignment 'context=(struct connectionRecType*)std::malloc(sizeof(struct connectionRecType))', assigned value is 0
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
            ^
src/emc/usr_intf/schedrmt.cc:1164:3: note: Null pointer dereference
  context->commMode = 0;
  ^
src/emc/usr_intf/schedrmt.cc:1165:3: warning: If memory allocation fails, then there is a possible null pointer dereference: context [nullPointerOutOfMemory]
  context->commProt = 0;
  ^
src/emc/usr_intf/schedrmt.cc:1156:41: note: Assuming allocation function fails
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
                                        ^
src/emc/usr_intf/schedrmt.cc:1156:13: note: Assignment 'context=(struct connectionRecType*)std::malloc(sizeof(struct connectionRecType))', assigned value is 0
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
            ^
src/emc/usr_intf/schedrmt.cc:1165:3: note: Null pointer dereference
  context->commProt = 0;
  ^
src/emc/usr_intf/schedrmt.cc:1166:3: warning: If memory allocation fails, then there is a possible null pointer dereference: context [nullPointerOutOfMemory]
  context->inBuf[0] = 0;
  ^
src/emc/usr_intf/schedrmt.cc:1156:41: note: Assuming allocation function fails
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
                                        ^
src/emc/usr_intf/schedrmt.cc:1156:13: note: Assignment 'context=(struct connectionRecType*)std::malloc(sizeof(struct connectionRecType))', assigned value is 0
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
            ^
src/emc/usr_intf/schedrmt.cc:1166:3: note: Null pointer dereference
  context->inBuf[0] = 0;
  ^
src/emc/usr_intf/schedrmt.cc:1170:16: warning: If memory allocation fails, then there is a possible null pointer dereference: context [nullPointerOutOfMemory]
    len = read(context->cliSock, &str, 1600);
               ^
src/emc/usr_intf/schedrmt.cc:1156:41: note: Assuming allocation function fails
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
                                        ^
src/emc/usr_intf/schedrmt.cc:1156:13: note: Assignment 'context=(struct connectionRecType*)std::malloc(sizeof(struct connectionRecType))', assigned value is 0
  context = (connectionRecType *) malloc(sizeof(connectionRecType));
            ^
src/emc/usr_intf/schedrmt.cc:1170:16: note: Null pointer dereference
    len = read(context->cliSock, &str, 1600);

[BsAtHome]

The allocation is done at the start of the thread readClient() started when a new connection is accepted. The "proper" action should be to check for NULL after allocation.

However,...

Now that I look at this code, there is a race condition on global var client_sockfd. The thread reads it, but it may already have been changed by the socket accept() code that spawns the thread (lines 1212..1217). The /proper/ solution is to allocate the connectionRecType structure in the accept() code, fill in the socket descriptor and pass the pointer to the thread as its argument.

[rmu75]

Addressing the broader point: the realtime stuff should be safe, it is supposed to be in locked memory that stays resident no matter what. So ESTOP and all related to that should work just fine. Running g-code program could eventually run out of parsed instructions but that should also be safe in the sense it shouldn't cause uncontrolled motion.

The linux kernel has a OOM killer that is supposed to get rid of processes that consume excessive memory and cause systemwide memory shortages. It wouldn't hurt if we checked with current distributions and realtime kernel how it reacts to OOM situations and if some tuning of the OOM killer would be in order. That kind of testing is probably hard to automate.

[BsAtHome]

Addressing the broader point: the realtime stuff should be safe [snip]

Yes, indeed. However, that is a separate story and not (directly) related to this issue.