Skip to content

Crash when long string pointer is nil #53

@MarkRivers

Description

@MarkRivers

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?

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions