dbPutString can leave strings unterminated

Attempted a fix with http://bazaar.launchpad.net/~epics-core/epics-base/3.14/revision/12620. This moves the field size test up before the string copy. Also made the change to "size-1".

Bruce could you please verify that this fixes the crash you see? If it doesn't please attach a .db file demonstrating the issue. This is clearly a bug, but from a quick test I couldn't actually cause a crash.

Bruce, how did you trigger this bug — by setting DOL to a too-long string in a .db file, or did the string come from some device support at runtime?

Setting a too-long DOL string triggers a different strcpy() buffer overflow in recGblInitConstantLink() for me, which Michael's commit doesn't fix.

I can't replicate your problem by setting the VAL field directly in a .db file, that gets rejected when the database is loaded. The other place where we call dbPutString() is in dbPutFieldLink() but that seems to protected already, my attempts to caput a too-long string to a link field get ignored (without reporting any errors anywhere though).

I just committed a fix for recGblInitConstantLink() to the 3.14 branch, it's the one after Micheal's.

Yes, Michael's patch fixes it and I can confirm the bug still
exists if I ifdef back to the prior version of dbStaticLib.c

I'm not sure why I haven't seen this before, as I'm sure I've
run into this issue of the stringin or stringout initializer being
over 40 characters before.

Perhaps we haven't see this problem before as the memory segment that
the record gets created in is probably all zeroes in many cases.

In one test yesterday I left the string unterminated by using the
existing dbStaticLib.c code, but changed the stringin init_record()
to use strncpy. It didn't crash, but a dbpr showed VAL as
arvScreens/Basler_acA1300-30gm-features.arvScreens/Basler_acA1300-30gm-features.
w/ OVAL as
arvScreens/Basler_acA1300-30gm-features.

The db file I tested w/ is just this, loaded after my usual db files:
record(stringin, "$(P)$(R)CamModelScreen") {
   field(VAL, "arvScreens/Basler_acA1300-30gm-features.edl")
   field(PINI, "YES")
}

The IOC is based on 3.14.12.4, w/ a dynamically linked RHEL5 executable
using ADCore, ffmpegServer, ADProsilica, aravisGigE, and quite a few other
related modules including our EVR module. The host is running kernel
2.6.18-164.15.1.el5 #1 SMP and has several other IOC's running on it.
Most of our ioc's have used static linking, so perhaps the dynamic linking
in this ioc makes a difference, as we've been using this kernel for years.

I can confirm that I get an error msg on dbLoadRecords:
dbLoadRecords( db/j.db "P=TST:GIGE:BASLER2,R=:,PORT=CAM" )
Can't set "TST:GIGE:BASLER2:CamModelScreen.VAL" to "arvScreens/Basler_acA1300-30gm-features.edl"
Error at or before ")" in file "db/j.db" line 3

but the unterminated val array has already been created even though it doesn't crash
until the iocInit() call.

Regards,
- Bruce

--
Bruce Hill
Member Technical Staff
SLAC National Accelerator Lab
2575 Sand Hill Road M/S 10
Menlo Park, CA 94025

Thanks Bruce, that makes sense now. I just posted a patch to the 3.14.12 Known Problems page which includes both Michael's and my fixes.