patch from Stefan Kratochwil <entwicklung@inovel.de> : canfestival-3-fm3_698.patch
authorfbeaulier
Mon, 29 Aug 2011 17:44:49 +0200
changeset 666 9febdd6fdc71
parent 665 90e6cf84a0d7
child 667 f48424ce2a5e
patch from Stefan Kratochwil <entwicklung@inovel.de> : 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!
src/objacces.c
src/sdo.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,
--- 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,