Experimental Physics and
Industrial Control System

<[email protected]> · Sat, 8 Nov 2014 15:59:00 +0000

Hi Mark,

I noticed that interruptCallbackWaveform() will add a NULL terminator if less than "nelm" characters are copied, but callbackWfRead() etc. do not do this. I enclose a patch that adds this feature to them too, and also a couple of "paranoid" string termination checks elsewhere that may or may not be useful/necessary

Regards,

Freddie

> -----Original Message-----
> From: Mark Rivers [mailto:[email protected]]
> Sent: 07 November 2014 23:34
> To: Akeroyd, Freddie (STFC,RAL,ISIS); '[email protected]'
> Subject: RE: Asyn and stringin records
> 
> As I was fixing the string termination problem that Freddie found, I
> discovered that there was another problem with interruptCallbackSi and
> interruptCallbackWaveform in devAsynOctet.c.  Neither of those functions
> was locking the record with dbScanLock while the record fields were being
> modified.  All of the other asyn device support does correctly lock records
> during the interrupt callbacks.  The problem was only in devAsynOctet.c.
> 
> I have fixed both problems and committed to SVN.  The fixes will be in the
> R4-25 release of asyn.  Meanwhile you can get them here:
> 
> https://svn.aps.anl.gov/epics/asyn/trunk
> 
> Mark
> 
> 
> 
> 
> -----Original Message-----
> From: Mark Rivers
> Sent: Friday, November 07, 2014 10:26 AM
> To: '[email protected]'; [email protected]
> Subject: RE: Asyn and stringin records
> 
> Hi Freddie,
> 
> This is the current code that handles the interrupt callbacks for the stringin
> record.  You are definitely correct that it does not guaranteed that the string
> is null terminated.
> 
> This is the code for periodic reads. It does guarantee that the string is null
> terminated.
> 
> static void callbackSiRead(asynUser *pasynUser)
> {
>     devPvt         *pdevPvt = (devPvt *)pasynUser->userPvt;
>     stringinRecord *psi = (stringinRecord *)pdevPvt->precord;
>     size_t         nBytesRead;
>     asynStatus     status;
>     size_t         len = sizeof(psi->val);
> 
>     status = readIt(pasynUser,psi->val,len,&nBytesRead);
>     psi->time = pasynUser->timestamp;
>     if(status==asynSuccess) {
>         psi->udf = 0;
>         if(nBytesRead==len) nBytesRead--;
>         psi->val[nBytesRead] = 0;
>     }
>     finish((dbCommon *)psi);
> }
> 
> This is the code for I/O Intr scanned records. It does NOT guarantee that the
> string is null terminated.
> 
> static void interruptCallbackSi(void *drvPvt, asynUser *pasynUser,
>        char *data,size_t numchars, int eomReason)
> {
>     devPvt         *pdevPvt = (devPvt *)drvPvt;
>     stringinRecord *psi = (stringinRecord *)pdevPvt->precord;
>     size_t         num;
> 
>     pdevPvt->gotValue = 1;
>     num = (numchars>=MAX_STRING_SIZE ? MAX_STRING_SIZE : numchars);
>     strncpy(psi->val,data,num);
>     psi->udf = 0;
>     if(num<MAX_STRING_SIZE) psi->val[num] = 0;
>     /* Set the status from pasynUser->auxStatus so I/O Intr scanned records
> can set alarms */
>     if (pdevPvt->status == asynSuccess) pdevPvt->status = pasynUser-
> >auxStatus;
>     psi->time = pasynUser->timestamp;
>     scanIoRequest(pdevPvt->ioScanPvt);
> }
> 
> I propose to change this to:
> 
> static void interruptCallbackSi(void *drvPvt, asynUser *pasynUser,
>        char *data,size_t numchars, int eomReason)
> {
>     devPvt         *pdevPvt = (devPvt *)drvPvt;
>     stringinRecord *psi = (stringinRecord *)pdevPvt->precord;
>     size_t maxChars = sizeof(psi->val)-1;
> 
>     pdevPvt->gotValue = 1;
>     if (numchars > maxChars) numchars = maxChars;
>     strncpy(psi->val,data,numchars);
>     psi->val[numchars] = 0;
>     psi->udf = 0;
>     /* Set the status from pasynUser->auxStatus so I/O Intr scanned records
> can set alarms */
>     if (pdevPvt->status == asynSuccess) pdevPvt->status = pasynUser-
> >auxStatus;
>     psi->time = pasynUser->timestamp;
>     scanIoRequest(pdevPvt->ioScanPvt);
> }
> 
> Mark
> 
> -----Original Message-----
> From: [email protected] [mailto:tech-talk-
> [email protected]] On Behalf Of [email protected]
> Sent: Friday, November 07, 2014 9:58 AM
> To: [email protected]
> Subject: Asyn and stringin records
> 
> Hi,
> 
> I have come across a difference in behaviour with asyn and the EPICS
> stringin record (asyn 4-22 / base 3.14.12.2 / win64) that occurs when the
> value received by the driver happens to be larger than the stringin
> maximum size. A record was changed from "periodic scan" to "I/O Intr"
> resulting in a crash due to a strcpy() (in monitor() from stringinRecord.c of
> base) being unable to find a NULL terminator and thus overwriting memory.
> Looking at devAsynOctet.c  it seems that stringin records that are
> periodically scanned are always guaranteed to have a NULL terminated
> value, whereas ones processed via interruptCallbackSi() are NULL
> terminated only if the data received is less than 40 characters long (I noticed
> that in base 3.14.12.4 the stringinRecord.c monitor() routine now uses
> strncpy() and so does not require NULL termination). For my case I've just
> added NULL termination for "I/O Intr" too, but from looking at the stringin
> record reference manual page I wasn't sure if the ori!
>  ginal I/O Instr stringin behaviour was actually more correct?
> 
> Regards,
> 
> Freddie
> 
> --
> Scanned by iCritical.

-- 
Scanned by iCritical.

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System