RE: Asyn and stringin records

[Mark Rivers <rivers@cars.uchicago.edu>]

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: tech-talk-bounces@aps.anl.gov [mailto:tech-talk-bounces@aps.anl.gov] On Behalf Of freddie.akeroyd@stfc.ac.uk
Sent: Friday, November 07, 2014 9:58 AM
To: tech-talk@aps.anl.gov
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.