EPICS Base

macExpandString writes out of boundary

Bug #551909 reported by Ralph Lange on 2010-03-30

This bug affects 1 person

Affects		Status	Importance	Assigned to	Milestone
	EPICS Base	Fix Released	Medium	Eric Norum

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

Tags:

Related branches

lp:~epics-core/epics-base/3.14

Andrew Johnson (anj) on 2010-03-30

tags:

added: codeathon

Revision history for this message

Eric Norum (wenorum) wrote on 2010-03-31:

With all this morning's correspondence in mind, here's a patch that I think fixes and clarifies things. The Application Developer's Guide should be changed to note the change in argument name (from maxlen to capacity). The AppDevGuide comment before macExpandString should be modified to add the note that the return value count does not include any trailing null.

Revision history for this message

Eric Norum (wenorum) wrote on 2010-03-31:

macLib.patch Edit (10.9 KiB, text/plain)

I think that all calls to macExpandString from base are now consistent.
The msi extension needs the patch:
--- msi.c.orig 2010-03-31 10:31:04.000000000 -0700
+++ msi.c 2010-03-31 10:31:07.000000000 -0700
@@ -227,7 +227,7 @@
  }
endif:
  if (expand) {
- n = macExpandString(macPvt,input,buffer,MAX_BUFFER_SIZE-1);
+ n = macExpandString(macPvt,input,buffer,MAX_BUFFER_SIZE);
      fputs(buffer,stdout);
      if (!unexpWarned && n<0) {
   fprintf(stderr,"Warning: unexpanded macros in ouput\n");

Revision history for this message

Andrew Johnson (anj) wrote on 2010-03-31:

I think msi.c should stay as it is, since we can't guarantee which version of Base it will be built against. The original msi.c is safe if built against the modified version of Base, but the reverse would not be the case.

Changed in epics-base:
importance:	Undecided → Medium
status:	New → In Progress
assignee:	nobody → Eric Norum (wenorum)

Revision history for this message

Andrew Johnson (anj) wrote on 2010-03-31:

See the core-talk thread at http://www.aps.anl.gov/epics/core-talk/2010/msg00013.php for earlier discussions.

Revision history for this message

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

On Mar 31, 2010, at 11:01 AM, Andrew Johnson wrote:

> I think msi.c should stay as it is, since we can't guarantee which
> version of Base it will be built against. The original msi.c is safe if
> built against the modified version of Base, but the reverse would not be
> the case.

Good point.

--
Eric Norum
<email address hidden>

Revision history for this message

Eric Norum (wenorum) wrote on 2010-04-01:

Adds overflow checks to macLibTest.c Edit (1.5 KiB, text/plain)

This patch adds some overflow checks to base/src/libCom/test/macLibTest.c

Andrew Johnson (anj) on 2010-05-04

Changed in epics-base:
status:	In Progress → Fix Committed

Revision history for this message

Eric Norum (wenorum) wrote on 2010-05-04:

Download full text (8.2 KiB)

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

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

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

Revision history for this message

Andrew Johnson (anj) wrote on 2010-05-04:

This was me noting that I committed the patch that you came up with that *was* "the other way", renaming "maxlen" to "capacity" etc. The change we agreed not to do was to modify msi.c. Sorry it took me a while to get around to applying your changes.

Andrew Johnson (anj) on 2010-11-24

Changed in epics-base:
status:	Fix Committed → Fix Released

Revision history for this message

Eric Norum (wenorum) wrote on 2010-11-24:

Download full text (8.2 KiB)

Didn't we deal with this a long time ago?
Or are you just catching up on old reports?

On Nov 24, 2010, at 2:05 PM, Andrew Johnson wrote:

Didn't we deal with this a long time ago?
Or are you just catching up on old reports?

On Nov 24, 2010, at 2:05 PM, Andrew Johnson wrote:

> ** Changed in: epics-base
>       Status: Fix Committed => Fix Released
> 
> -- 
> 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 Released
> 
> 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

Report a bug

This report contains Public information

Everyone can see this information.

You are

Subscribing...

Edit bug mail

Other bug subscribers

Patches

Add patch

Remote bug watches

Bug watches keep track of this bug in other bug trackers.