Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when long string pointer is nil #53

Open
MarkRivers opened this issue Feb 15, 2023 · 0 comments
Open

Crash when long string pointer is nil #53

MarkRivers opened this issue Feb 15, 2023 · 0 comments
Assignees

Comments

@MarkRivers
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants