EPICS Controls Argonne National Laboratory

Experimental Physics and
Industrial Control System

1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  <20142015  2016  2017  2018  2019  2020  2021  2022  2023  2024  Index 1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  <20142015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
<== Date ==> <== Thread ==>

Subject: RE: Asyn and stringin records
From: Mark Rivers <[email protected]>
To: "'[email protected]'" <[email protected]>, "'[email protected]'" <[email protected]>
Date: Fri, 7 Nov 2014 23:33:55 +0000
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:[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.



Replies:
RE: Asyn and stringin records freddie.akeroyd
References:
Asyn and stringin records freddie.akeroyd
RE: Asyn and stringin records Mark Rivers

Navigate by Date:
Prev: Re: Preemptive CA client that never calls ca_flush_io, ca_pend_io, ca_pend_event, nor ca_poll Andrew Johnson
Next: RE: Asyn and stringin records freddie.akeroyd
Index: 1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  <20142015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
Navigate by Thread:
Prev: RE: Asyn and stringin records Mark Rivers
Next: RE: Asyn and stringin records freddie.akeroyd
Index: 1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  <20142015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
ANJ, 17 Dec 2015 Valid HTML 4.01! · Home · News · About · Base · Modules · Extensions · Distributions · Download ·
· Search · EPICS V4 · IRMIS · Talk · Bugs · Documents · Links · Licensing ·