# HG changeset patch # User fbeaulier # Date 1314632689 -7200 # Node ID 9febdd6fdc71c927df6dfbd5a059c4ec302acc81 # Parent 90e6cf84a0d78b507158305627ac34cba401ad0b patch from Stefan Kratochwil : canfestival-3-fm3_698.patch Bug: If an object dictionary entry was requested whose size exceeds SDO_MAX_LENGTH_TRANSFERT, the memcpy() call at line 139 of objacces.c overwrites the memory after *pDestData which causes stack corruption. -> Bugfix: The bug was corrected by size checking the requested data. An 'Out of memory' error message will be generated if the requested data exceeds SDO_MAX_LENGTH_TRANSFERT. Additional changes: Added dynamic buffer allocation for the SDO transfer. This feature can be used if SDO_DYNAMIC_BUFFER_ALLOCATION is defined in config.h. The size of the dynamically allocated buffer is controlled with SDO_DYNAMIC_BUFFER_ALLOCATION_SIZE. -> Note: This change removes the detection of OD_LENGTH_DATA_INVALID errors! diff -r 90e6cf84a0d7 -r 9febdd6fdc71 src/objacces.c --- a/src/objacces.c Mon Aug 29 17:31:55 2011 +0200 +++ b/src/objacces.c Mon Aug 29 17:44:49 2011 +0200 @@ -110,63 +110,63 @@ return OD_READ_NOT_ALLOWED; } + if (pDestData == 0) { + return SDOABT_GENERAL_ERROR; + } + + if (ptrTable->pSubindex[bSubindex].size > *pExpectedSize) { + /* Requested variable is too large to fit into a transfer line, inform * + * the caller about the real size of the requested variable. */ + *pExpectedSize = ptrTable->pSubindex[bSubindex].size; + return SDOABT_OUT_OF_MEMORY; + } + *pDataType = ptrTable->pSubindex[bSubindex].bDataType; szData = ptrTable->pSubindex[bSubindex].size; - if(*pExpectedSize == 0 || - *pExpectedSize == szData || - /* allow to fetch a shorter string than expected */ - (*pDataType >= visible_string && *pExpectedSize < szData)) { - # ifdef CANOPEN_BIG_ENDIAN - if(endianize && *pDataType > boolean && !( - *pDataType >= visible_string && - *pDataType <= domain)) { - /* data must be transmited with low byte first */ - UNS8 i, j = 0; - MSG_WAR(boolean, "data type ", *pDataType); - MSG_WAR(visible_string, "data type ", *pDataType); - for ( i = szData ; i > 0 ; i--) { - MSG_WAR(i," ", j); - ((UNS8*)pDestData)[j++] = - ((UNS8*)ptrTable->pSubindex[bSubindex].pObject)[i-1]; + if(endianize && *pDataType > boolean && !( + *pDataType >= visible_string && + *pDataType <= domain)) { + /* data must be transmited with low byte first */ + UNS8 i, j = 0; + MSG_WAR(boolean, "data type ", *pDataType); + MSG_WAR(visible_string, "data type ", *pDataType); + for ( i = szData ; i > 0 ; i--) { + MSG_WAR(i," ", j); + ((UNS8*)pDestData)[j++] = + ((UNS8*)ptrTable->pSubindex[bSubindex].pObject)[i-1]; + } + *pExpectedSize = szData; + } + else /* no endianisation change */ +# endif + + if(*pDataType != visible_string) { + memcpy(pDestData, ptrTable->pSubindex[bSubindex].pObject,szData); + *pExpectedSize = szData; + }else{ + /* TODO : CONFORM TO DS-301 : + * - stop using NULL terminated strings + * - store string size in td_subindex + * */ + /* Copy null terminated string to user, and return discovered size */ + UNS8 *ptr = (UNS8*)ptrTable->pSubindex[bSubindex].pObject; + UNS8 *ptr_start = ptr; + /* *pExpectedSize IS < szData . if null, use szData */ + UNS8 *ptr_end = ptr + (*pExpectedSize ? *pExpectedSize : szData) ; + UNS8 *ptr_dest = (UNS8*)pDestData; + while( *ptr && ptr < ptr_end){ + *(ptr_dest++) = *(ptr++); } - *pExpectedSize = szData; - } - else /* no endianisation change */ -# endif - if(*pDataType != visible_string) { - memcpy(pDestData, ptrTable->pSubindex[bSubindex].pObject,szData); - *pExpectedSize = szData; - }else{ - /* TODO : CONFORM TO DS-301 : - * - stop using NULL terminated strings - * - store string size in td_subindex - * */ - /* Copy null terminated string to user, and return discovered size */ - UNS8 *ptr = (UNS8*)ptrTable->pSubindex[bSubindex].pObject; - UNS8 *ptr_start = ptr; - /* *pExpectedSize IS < szData . if null, use szData */ - UNS8 *ptr_end = ptr + (*pExpectedSize ? *pExpectedSize : szData) ; - UNS8 *ptr_dest = (UNS8*)pDestData; - while( *ptr && ptr < ptr_end){ - *(ptr_dest++) = *(ptr++); - } - - *pExpectedSize = (UNS32) (ptr - ptr_start); - /* terminate string if not maximum length */ - if (*pExpectedSize < szData) - *(ptr) = 0; - } - - return OD_SUCCESSFUL; - } - else { /* Error ! */ - *pExpectedSize = szData; - accessDictionaryError(wIndex, bSubindex, szData, - *pExpectedSize, OD_LENGTH_DATA_INVALID); - return OD_LENGTH_DATA_INVALID; - } + + *pExpectedSize = (UNS32) (ptr - ptr_start); + /* terminate string if not maximum length */ + if (*pExpectedSize < szData) + *(ptr) = 0; + } + + return OD_SUCCESSFUL; } UNS32 _setODentry( CO_Data* d, diff -r 90e6cf84a0d7 -r 9febdd6fdc71 src/sdo.c --- a/src/sdo.c Mon Aug 29 17:31:55 2011 +0200 +++ b/src/sdo.c Mon Aug 29 17:44:49 2011 +0200 @@ -239,7 +239,7 @@ **/ UNS32 objdictToSDOline (CO_Data* d, UNS8 line) { - UNS32 size = 0; + UNS32 size = SDO_MAX_LENGTH_TRANSFERT; UNS8 dataType; UNS32 errorCode; @@ -247,11 +247,28 @@ MSG_WAR(0x3A06, " subIndex : ", d->transfers[line].subIndex); #ifdef SDO_DYNAMIC_BUFFER_ALLOCATION - //TODO: Read the size of the object. Depending o it put data into data or dynamicData + /* Try to use the static buffer. */ errorCode = getODentry(d, d->transfers[line].index, d->transfers[line].subIndex, (void *)d->transfers[line].data, &size, &dataType, 1); + if (errorCode == SDOABT_OUT_OF_MEMORY) { + /* The static buffer is too small, try again using a dynamic buffer. * + * 'size' now contains the real size of the requested object. */ + if (size <= SDO_DYNAMIC_BUFFER_ALLOCATION_SIZE) { + d->transfers[line].dynamicData = (UNS8 *) malloc(size * sizeof(UNS8)); + if (d->transfers[line].dynamicData != NULL) { + d->transfers[line].dynamicDataSize = size; + errorCode = getODentry(d, + d->transfers[line].index, + d->transfers[line].subIndex, + (void *) d->transfers[line].dynamicData, + &d->transfers[line].dynamicDataSize, + &dataType, + 1); + } + } + } #else //SDO_DYNAMIC_BUFFER_ALLOCATION errorCode = getODentry(d, d->transfers[line].index, d->transfers[line].subIndex,