search

epics-modules / autosave

APS BCDA synApps module: autosave
https://epics-modules.github.io/autosave/
Other
8 stars 31 forks source link

Crash when long string pointer is nil #53

Open MarkRivers opened 1 year ago

MarkRivers commented 1 year ago

My IOC was crashing in strNcpy during the autosave operation in line 1884 in save_restore.c

I added this debugging line:

diff --git a/asApp/src/save_restore.c b/asApp/src/save_restore.c
index 526c551..1748d21 100644
--- a/asApp/src/save_restore.c
+++ b/asApp/src/save_restore.c
@@ -1881,6 +1881,8 @@ STATIC int write_it(char *filename, struct chlist *plist)
                errno = 0;
                if (is_long_string) {
                        /* write first BUF-SIZE-1 characters of long string, so dbrestore doesn't choke. */
+printf("save_restore.c::write_it, name=%s, value_string=%p, pchannel=%p, pchannel->pArray=%p, pchannel->pArray=%s, BUF_SIZE=%d\n",
+                                  pchannel->name, value_string, pchannel, pchannel->pArray, pchannel->pArray, BUF_SIZE);
                        strNcpy(value_string, pchannel->pArray, BUF_SIZE);
                        value_string[BUF_SIZE-1] = '\0';
                        n = epicsStrPrintEscaped(out_fd, value_string, strlen(value_string));

I then saw this output before it crashed.

save_restore.c::write_it, name=13IDA_TEST:DMM1Ch1_calc.CALC$, value_string=0x7f6bbf7f24f0, pchannel=0x3365ba0, pchannel->pArray=(nil), pchannel->pArray=(null), BUF_SIZE=200

So the problem is that pchannel->pArray is nil, and strNCpy crashes because it dereferences the nil pointer.

I added a check in strNcpy in save_restore.h to protect against nil destination or source pointers:

diff --git a/asApp/src/save_restore.h b/asApp/src/save_restore.h
index 8f25e82..e9d4b8c 100644
--- a/asApp/src/save_restore.h
+++ b/asApp/src/save_restore.h
@@ -116,8 +116,9 @@ extern float mySafeDoubleToFloat(double d);
        int ii;                                                         \
        char *dd=dest;                                          \
        const char *ss=src;                                     \
-       for (ii=0; *ss && ii < N-1; ii++)       \
-               *dd++ = *ss++;                                  \
+       if (dd && ss) \
+         for (ii=0; *ss && ii < N-1; ii++)     \
+                 *dd++ = *ss++;                                        \
        *dd = '\0';                                                     \
 }

I then see this and it does not crash:

save_restore.c::write_it, name=13IDA_TEST:DMM1Ch1_calc.OCAL$, value_string=0x7f6bbf7f24f0, pchannel=0x3365c70, pchannel->pArray=(nil), pchannel->pArray=(null), BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:DMM1Ch2_calc.CALC$, value_string=0x7f6bbf7f24f0, pchannel=0x3367d90, pchannel->pArray=(nil), pchannel->pArray=(null), BUF_SIZE=200
...
save_restore.c::write_it, name=13IDA_TEST:userTran9.CLCO$, value_string=0x7f6bbf7f24f0, pchannel=0x345e170, pchannel->pArray=0x7f6bac15e580, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran9.CLCP$, value_string=0x7f6bbf7f24f0, pchannel=0x345e240, pchannel->pArray=0x7f6bac15e600, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran10.CLCA$, value_string=0x7f6bbf7f24f0, pchannel=0x3461c30, pchannel->pArray=0x7f6bac15e680, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran10.CLCB$, value_string=0x7f6bbf7f24f0, pchannel=0x3461d00, pchannel->pArray=0x7f6bac15e700, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran10.CLCC$, value_string=0x7f6bbf7f24f0, pchannel=0x3461dd0, pchannel->pArray=0x7f6bac15e780, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran10.CLCD$, value_string=0x7f6bbf7f24f0, pchannel=0x3461ea0, pchannel->pArray=0x7f6bac15e800, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran10.CLCE$, value_string=0x7f6bbf7f24f0, pchannel=0x3461f70, pchannel->pArray=0x7f6bac15e880, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran10.CLCF$, value_string=0x7f6bbf7f24f0, pchannel=0x3462040, pchannel->pArray=0x7f6bac15e900, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran10.CLCG$, value_string=0x7f6bbf7f24f0, pchannel=0x3462110, pchannel->pArray=0x7f6bac15e980, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran10.CLCH$, value_string=0x7f6bbf7f24f0, pchannel=0x34621e0, pchannel->pArray=0x7f6bac15ea00, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran10.CLCI$, value_string=0x7f6bbf7f24f0, pchannel=0x34622b0, pchannel->pArray=0x7f6bac15ea80, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran10.CLCJ$, value_string=0x7f6bbf7f24f0, pchannel=0x3462380, pchannel->pArray=0x7f6bac15eb00, pchannel->pArray=, BUF_SIZE=200
save_restore.c::write_it, name=13IDA_TEST:userTran10.CLCK$, value_string=0x7f6bbf7f24f0, pchannel=0x3462450, pchannel->pArray=0x7f6bac15eb80, pchannel->pArray=, BUF_SIZE=200

That fixes the problem but does not explain why that pointer can be nil in the first place?