Skip to content
Snippets Groups Projects
Commit e7642321 authored by Rob Swindell's avatar Rob Swindell :speech_balloon:
Browse files

Fix getnodedat() error handling and usage

If a file descriptor is passed to getnodedat() and the lock retry counter was
reached, the file would be closed, but the passed file descriptor reference
would not be set to -1. This could result in exceptions (from subsequent read
attempts on the referenced file descriptor) in cases where the node.dab could
not be locked or read by getnodedat() and was thus closed.

The set/get_node_* helper functions (used by MQTT) were not initializing the
node.dab file descriptor (i.e. to -1), so it's possible getnodedat() could
try to read from and close an invalid/wrong open file descriptor. If the local
variable happened to be initialized to a value <= 0, then, no problem, but
this is undefined behavior (UB).
parent 6d7b60d0
No related branches found
No related tags found
1 merge request!455Update branch with changes from master
Pipeline #6477 passed
...@@ -1149,8 +1149,8 @@ int getnodedat(scfg_t* cfg, uint number, node_t *node, bool lockit, int* fdp) ...@@ -1149,8 +1149,8 @@ int getnodedat(scfg_t* cfg, uint number, node_t *node, bool lockit, int* fdp)
} }
if(fdp==NULL || count==LOOP_NODEDAB) if(fdp==NULL || count==LOOP_NODEDAB)
close(file); CLOSE_OPEN_FILE(file);
else if(fdp!=NULL)
*fdp=file; *fdp=file;
if(count==LOOP_NODEDAB) if(count==LOOP_NODEDAB)
...@@ -1200,7 +1200,7 @@ int putnodedat(scfg_t* cfg, uint number, node_t* node, bool closeit, int file) ...@@ -1200,7 +1200,7 @@ int putnodedat(scfg_t* cfg, uint number, node_t* node, bool closeit, int file)
bool set_node_status(scfg_t* cfg, int nodenum, enum node_status status) bool set_node_status(scfg_t* cfg, int nodenum, enum node_status status)
{ {
node_t node; node_t node;
int file; int file = -1;
if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0) if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0)
return false; return false;
...@@ -1213,7 +1213,7 @@ bool set_node_status(scfg_t* cfg, int nodenum, enum node_status status) ...@@ -1213,7 +1213,7 @@ bool set_node_status(scfg_t* cfg, int nodenum, enum node_status status)
bool set_node_misc(scfg_t* cfg, int nodenum, uint misc) bool set_node_misc(scfg_t* cfg, int nodenum, uint misc)
{ {
node_t node; node_t node;
int file; int file = -1;
if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0) if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0)
return false; return false;
...@@ -1226,7 +1226,7 @@ bool set_node_misc(scfg_t* cfg, int nodenum, uint misc) ...@@ -1226,7 +1226,7 @@ bool set_node_misc(scfg_t* cfg, int nodenum, uint misc)
bool set_node_lock(scfg_t* cfg, int nodenum, bool set) bool set_node_lock(scfg_t* cfg, int nodenum, bool set)
{ {
node_t node; node_t node;
int file; int file = -1;
if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0) if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0)
return false; return false;
...@@ -1242,7 +1242,7 @@ bool set_node_lock(scfg_t* cfg, int nodenum, bool set) ...@@ -1242,7 +1242,7 @@ bool set_node_lock(scfg_t* cfg, int nodenum, bool set)
bool set_node_interrupt(scfg_t* cfg, int nodenum, bool set) bool set_node_interrupt(scfg_t* cfg, int nodenum, bool set)
{ {
node_t node; node_t node;
int file; int file = -1;
if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0) if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0)
return false; return false;
...@@ -1258,7 +1258,7 @@ bool set_node_interrupt(scfg_t* cfg, int nodenum, bool set) ...@@ -1258,7 +1258,7 @@ bool set_node_interrupt(scfg_t* cfg, int nodenum, bool set)
bool set_node_down(scfg_t* cfg, int nodenum, bool set) bool set_node_down(scfg_t* cfg, int nodenum, bool set)
{ {
node_t node; node_t node;
int file; int file = -1;
if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0) if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0)
return false; return false;
...@@ -1274,7 +1274,7 @@ bool set_node_down(scfg_t* cfg, int nodenum, bool set) ...@@ -1274,7 +1274,7 @@ bool set_node_down(scfg_t* cfg, int nodenum, bool set)
bool set_node_rerun(scfg_t* cfg, int nodenum, bool set) bool set_node_rerun(scfg_t* cfg, int nodenum, bool set)
{ {
node_t node; node_t node;
int file; int file = -1;
if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0) if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0)
return false; return false;
...@@ -1290,7 +1290,7 @@ bool set_node_rerun(scfg_t* cfg, int nodenum, bool set) ...@@ -1290,7 +1290,7 @@ bool set_node_rerun(scfg_t* cfg, int nodenum, bool set)
bool set_node_errors(scfg_t* cfg, int nodenum, uint errors) bool set_node_errors(scfg_t* cfg, int nodenum, uint errors)
{ {
node_t node; node_t node;
int file; int file = -1;
if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0) if(getnodedat(cfg, nodenum, &node, /* lockit: */true, &file) != 0)
return false; return false;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment