EPICS Base

Bug #551909
Comment #7

Comment 7 for bug 551909

Revision history for this message

Eric Norum (wenorum) wrote on 2010-05-04: Re: [Bug 551909] Re: macExpandString writes out of boundary

I thought that we'd fixed this some other way.

On May 4, 2010, at 1:45 PM, Andrew Johnson wrote:

> ** Changed in: epics-base
> Status: In Progress => Fix Committed
>
> --
> macExpandString writes out of boundary
> https://bugs.launchpad.net/bugs/551909
> You received this bug notification because you are a bug assignee.
>
> Status in EPICS Base: Fix Committed
>
> Bug description:
> Andrew and others,
>
> I’ve just found a bug in libCom/macLib. I had a long command line (dbLoadRecords(…)) in a startup script, with several macros in it. When the string is expanded it is more than 256 characters, so macEnvExpand has to go through an extra iteration. When it calls free(dest), Windows (win32-x86 architecture) detects heap corruption. It says that values past the end of “dest” have been modified. This almost certainly means that macExpandString has written past the end of “dest”. I suspect it is by 1 character.
>
> To track it down I set handle->debug=3 in macEnvExpand, and got the following output.
>
>
> dbLoadRecords("J:/epics/devel/areaDetector-1-6/ADApp/Db/NDOverlay.template", "P=13SIM1:,R=Over1:, PORT=OVER1,ADDR=0,TIMEOUT=1")
> macExpandString( J:/epics/devel/areaDetector-1-6/ADApp/Db/NDOverlay.template, maxlen = 256 )
> trans-> entry = 0012FC60, level = 0, maxlen = 256, discard = F, rawval = J:/epics/devel/areaDetector-1-6/ADApp/Db/NDOverlay.template
> <-trans level = 0, length = 59, value = J:/epics/devel/areaDetector-1-6/ADApp/Db/NDOverlay.template
> macExpandString() -> 59
> macDeleteHandle()
> macExpandString( dbLoadRecords("$(AREA_DETECTOR)/ADApp/Db/NDOverlayN.template","P=$(PREFIX),R=Over1:1:,NAME=ROI1, SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1"), maxlen = 256 )
> trans-> entry = 0012FD10, level = 0, maxlen = 256, discard = F, rawval = dbLoadRecords("$(AREA_DETECTOR)/ADApp/Db/NDOverlayN.template","P=$(PREFIX),R=Over1:1:,NAME=ROI1, SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> refer-> entry = 0012FD10, level = 0, maxlen = 241, rawval = $(AREA_DETECTOR)/ADApp/Db/NDOverlayN.template","P=$(PREFIX),R=Over1:1:,NAME=ROI1, SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,POR
> T=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = AREA_DETECTOR)/ADApp/Db/NDOverlayN.template","P=$(PREFIX),R=Over1:1:,NAME=ROI1, SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length = 13, value = AREA_DETECTOR
> lookup-> level = 0, name = AREA_DETECTOR, special = 0
> <-lookup level = 0, name = AREA_DETECTOR, result = 010182A8
> trans-> entry = 0012FD10, level = 1, maxlen = 241, discard = T, rawval = J:/epics/devel/areaDetector-1-6
> <-trans level = 1, length = 31, value = J:/epics/devel/areaDetector-1-6
> <-refer level = 0, length = 31, value = J:/epics/devel/areaDetector-1-6
> refer-> entry = 0012FD10, level = 0, maxlen = 176, rawval = $(PREFIX),R=Over1:1:,NAME=ROI1, SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = PREFIX),R=Over1:1:,NAME=ROI1, SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length = 6, value = PREFIX
> lookup-> level = 0, name = PREFIX, special = 0
> <-lookup level = 0, name = PREFIX, result = 01038698
> trans-> entry = 0012FD10, level = 1, maxlen = 176, discard = T, rawval = 13SIM1:
> <-trans level = 1, length = 7, value = 13SIM1:
> <-refer level = 0, length = 7, value = 13SIM1:
> refer-> entry = 0012FD10, level = 0, maxlen = 122, rawval = $(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length = 6, value = PREFIX
> lookup-> level = 0, name = PREFIX, special = 0
> <-lookup level = 0, name = PREFIX, result = 01038698
> trans-> entry = 0012FD10, level = 1, maxlen = 122, discard = T, rawval = 13SIM1:
> <-trans level = 1, length = 7, value = 13SIM1:
> <-refer level = 0, length = 7, value = 13SIM1:
> refer-> entry = 0012FD10, level = 0, maxlen = 96, rawval = $(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length = 6, value = PREFIX
> lookup-> level = 0, name = PREFIX, special = 0
> <-lookup level = 0, name = PREFIX, result = 01038698
> trans-> entry = 0012FD10, level = 1, maxlen = 96, discard = T, rawval = 13SIM1:
> <-trans level = 1, length = 7, value = 13SIM1:
> <-refer level = 0, length = 7, value = 13SIM1:
> refer-> entry = 0012FD10, level = 0, maxlen = 69, rawval = $(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length = 6, value = PREFIX
> lookup-> level = 0, name = PREFIX, special = 0
> <-lookup level = 0, name = PREFIX, result = 01038698
> trans-> entry = 0012FD10, level = 1, maxlen = 69, discard = T, rawval = 13SIM1:
> <-trans level = 1, length = 7, value = 13SIM1:
> <-refer level = 0, length = 7, value = 13SIM1:
> refer-> entry = 0012FD10, level = 0, maxlen = 41, rawval = $(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length = 6, value = PREFIX
> lookup-> level = 0, name = PREFIX, special = 0
> <-lookup level = 0, name = PREFIX, result = 01038698
> trans-> entry = 0012FD10, level = 1, maxlen = 41, discard = T, rawval = 13SIM1:
> <-trans level = 1, length = 7, value = 13SIM1:
> <-refer level = 0, length = 7, value = 13SIM1:
> <-trans level = 0, length = 256, value = dbLoadRecords("J:/epics/devel/areaDetector-1-6/ADApp/Db/NDOverlayN.template","P=13SIM1:,R=Over1:1:,NAME=ROI1, SHAPE=1,O=Over1:,XPOS=13SIM1:ROI1:MinX_RBV,YPOS=13SIM1:ROI1:MinY_RBV,XSIZE=13SIM1:ROI1:SizeX_RBV,YSIZE=13SIM1:ROI1:SizeY_RBV,
> PORT=OVER1,ADDR=0,T
> macExpandString() -> 256
>
>
> I have not digested the output. To try to work around the problem, I modified the line:
>
> n = macExpandString(handle, str, dest, destCapacity);
>
> to be
>
> n = macExpandString(handle, str, dest, destCapacity-1);
>
> i.e. to tell macExpandString that the buffer is 1 character smaller than it really is.
>
> Index: macLib/macEnv.c
> ===================================================================
> RCS file: /net/phoebus/epicsmgr/cvsroot/epics/base/src/libCom/macLib/macEnv.c,v
> retrieving revision 1.2.2.6
> diff -u -r1.2.2.6 macEnv.c
> --- macLib/macEnv.c 23 Apr 2009 18:49:39 -0000 1.2.2.6
> +++ macLib/macEnv.c 30 Mar 2010 14:23:26 -0000
> @@ -38,7 +38,7 @@
> */
> free(dest);
> dest = mallocMustSucceed(destCapacity, "macEnvExpand");
> - n = macExpandString(handle, str, dest, destCapacity);
> + n = macExpandString(handle, str, dest, destCapacity-1);
> } while (n >= (destCapacity - 1));
>
> if (n < 0) {
>
>
> That fixed the problem, but I think that probably the correct solution is to find where macExpandString is writing past the end of dest and fixing that.
>
> Mark
>
>

--
Eric Norum
<email address hidden>

I thought that we'd fixed this some other way.

On May 4, 2010, at 1:45 PM, Andrew Johnson wrote:

> ** Changed in: epics-base
>       Status: In Progress => Fix Committed
> 
> -- 
> macExpandString writes out of boundary
> https://bugs.launchpad.net/bugs/551909
> You received this bug notification because you are a bug assignee.
> 
> Status in EPICS Base: Fix Committed
> 
> Bug description:
> Andrew and others,
> 
> I’ve just found a bug in libCom/macLib.  I had a long command line (dbLoadRecords(…)) in a startup script, with several macros in it.  When the string is expanded it is more than 256 characters, so macEnvExpand has to go through an extra iteration.  When it calls free(dest), Windows (win32-x86 architecture) detects heap corruption.  It says that values past the end of “dest” have been modified.  This almost certainly means that macExpandString has written past the end of “dest”.  I suspect it is by 1 character.
> 
> To track it down I set handle->debug=3 in macEnvExpand, and got the following output.
> 
> 
> dbLoadRecords("J:/epics/devel/areaDetector-1-6/ADApp/Db/NDOverlay.template",   "P=13SIM1:,R=Over1:, PORT=OVER1,ADDR=0,TIMEOUT=1")
> macExpandString( J:/epics/devel/areaDetector-1-6/ADApp/Db/NDOverlay.template, maxlen = 256 )
> trans-> entry = 0012FC60, level = 0, maxlen = 256, discard = F, rawval = J:/epics/devel/areaDetector-1-6/ADApp/Db/NDOverlay.template
> <-trans level = 0, length =   59, value  = J:/epics/devel/areaDetector-1-6/ADApp/Db/NDOverlay.template
> macExpandString() -> 59
> macDeleteHandle()
> macExpandString( dbLoadRecords("$(AREA_DETECTOR)/ADApp/Db/NDOverlayN.template","P=$(PREFIX),R=Over1:1:,NAME=ROI1, SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1"), maxlen = 256 )
> trans-> entry = 0012FD10, level = 0, maxlen = 256, discard = F, rawval = dbLoadRecords("$(AREA_DETECTOR)/ADApp/Db/NDOverlayN.template","P=$(PREFIX),R=Over1:1:,NAME=ROI1,   SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> refer-> entry = 0012FD10, level = 0, maxlen = 241, rawval = $(AREA_DETECTOR)/ADApp/Db/NDOverlayN.template","P=$(PREFIX),R=Over1:1:,NAME=ROI1,   SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,POR
> T=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = AREA_DETECTOR)/ADApp/Db/NDOverlayN.template","P=$(PREFIX),R=Over1:1:,NAME=ROI1,  SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length =   13, value  = AREA_DETECTOR
> lookup-> level = 0, name = AREA_DETECTOR, special = 0
> <-lookup level = 0, name = AREA_DETECTOR, result = 010182A8
> trans-> entry = 0012FD10, level = 1, maxlen = 241, discard = T, rawval = J:/epics/devel/areaDetector-1-6
> <-trans level = 1, length =   31, value  = J:/epics/devel/areaDetector-1-6
> <-refer level = 0, length =   31, value  = J:/epics/devel/areaDetector-1-6
> refer-> entry = 0012FD10, level = 0, maxlen = 176, rawval = $(PREFIX),R=Over1:1:,NAME=ROI1,   SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = PREFIX),R=Over1:1:,NAME=ROI1,   SHAPE=1,O=Over1:,XPOS=$(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length =    6, value  = PREFIX
> lookup-> level = 0, name = PREFIX, special = 0
> <-lookup level = 0, name = PREFIX, result = 01038698
> trans-> entry = 0012FD10, level = 1, maxlen = 176, discard = T, rawval = 13SIM1:
> <-trans level = 1, length =    7, value  = 13SIM1:
> <-refer level = 0, length =    7, value  = 13SIM1:
> refer-> entry = 0012FD10, level = 0, maxlen = 122, rawval = $(PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = PREFIX)ROI1:MinX_RBV,YPOS=$(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length =    6, value  = PREFIX
> lookup-> level = 0, name = PREFIX, special = 0
> <-lookup level = 0, name = PREFIX, result = 01038698
> trans-> entry = 0012FD10, level = 1, maxlen = 122, discard = T, rawval = 13SIM1:
> <-trans level = 1, length =    7, value  = 13SIM1:
> <-refer level = 0, length =    7, value  = 13SIM1:
> refer-> entry = 0012FD10, level = 0, maxlen = 96, rawval = $(PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = PREFIX)ROI1:MinY_RBV,XSIZE=$(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length =    6, value  = PREFIX
> lookup-> level = 0, name = PREFIX, special = 0
> <-lookup level = 0, name = PREFIX, result = 01038698
> trans-> entry = 0012FD10, level = 1, maxlen = 96, discard = T, rawval = 13SIM1:
> <-trans level = 1, length =    7, value  = 13SIM1:
> <-refer level = 0, length =    7, value  = 13SIM1:
> refer-> entry = 0012FD10, level = 0, maxlen = 69, rawval = $(PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = PREFIX)ROI1:SizeX_RBV,YSIZE=$(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length =    6, value  = PREFIX
> lookup-> level = 0, name = PREFIX, special = 0
> <-lookup level = 0, name = PREFIX, result = 01038698
> trans-> entry = 0012FD10, level = 1, maxlen = 69, discard = T, rawval = 13SIM1:
> <-trans level = 1, length =    7, value  = 13SIM1:
> <-refer level = 0, length =    7, value  = 13SIM1:
> refer-> entry = 0012FD10, level = 0, maxlen = 41, rawval = $(PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> trans-> entry = 0012FD10, level = 1, maxlen = 256, discard = T, rawval = PREFIX)ROI1:SizeY_RBV,PORT=OVER1,ADDR=0,TIMEOUT=1")
> <-trans level = 1, length =    6, value  = PREFIX
> lookup-> level = 0, name = PREFIX, special = 0
> <-lookup level = 0, name = PREFIX, result = 01038698
> trans-> entry = 0012FD10, level = 1, maxlen = 41, discard = T, rawval = 13SIM1:
> <-trans level = 1, length =    7, value  = 13SIM1:
> <-refer level = 0, length =    7, value  = 13SIM1:
> <-trans level = 0, length =  256, value  = dbLoadRecords("J:/epics/devel/areaDetector-1-6/ADApp/Db/NDOverlayN.template","P=13SIM1:,R=Over1:1:,NAME=ROI1,   SHAPE=1,O=Over1:,XPOS=13SIM1:ROI1:MinX_RBV,YPOS=13SIM1:ROI1:MinY_RBV,XSIZE=13SIM1:ROI1:SizeX_RBV,YSIZE=13SIM1:ROI1:SizeY_RBV,
> PORT=OVER1,ADDR=0,T
> macExpandString() -> 256
> 
> 
> I have not digested the output.  To try to work around the problem, I modified the line:
> 
>        n = macExpandString(handle, str, dest, destCapacity);
> 
> to be
> 
>        n = macExpandString(handle, str, dest, destCapacity-1);
> 
> i.e. to tell macExpandString that the buffer is 1 character smaller than it really is.
> 
> Index: macLib/macEnv.c
> ===================================================================
> RCS file: /net/phoebus/epicsmgr/cvsroot/epics/base/src/libCom/macLib/macEnv.c,v
> retrieving revision 1.2.2.6
> diff -u -r1.2.2.6 macEnv.c
> --- macLib/macEnv.c     23 Apr 2009 18:49:39 -0000      1.2.2.6
> +++ macLib/macEnv.c     30 Mar 2010 14:23:26 -0000
> @@ -38,7 +38,7 @@
>          */
>         free(dest);
>         dest = mallocMustSucceed(destCapacity, "macEnvExpand");
> -        n = macExpandString(handle, str, dest, destCapacity);
> +        n = macExpandString(handle, str, dest, destCapacity-1);
>     } while (n >= (destCapacity - 1));
> 
>     if (n < 0) {
> 
> 
> That fixed the problem, but I think that probably the correct solution is to find where macExpandString is writing past the end of dest and fixing that.
> 
> Mark
> 
>

-- 
Eric Norum
wenorum@lbl.gov