From 0965cac4fb1f4d26e240921a9a6257ecd8ec7e61 Mon Sep 17 00:00:00 2001
From: Florian Pose <fp@igh-essen.com>
Date: Thu, 20 Sep 2007 09:12:27 +0000
Subject: [PATCH] Improved checking of EEPROM data while reading.

---
 master/fsm_slave.c | 64 +++++++++++++++++++++++++++++++++-------------
 master/slave.c     | 46 +++++++++++++++++++++++----------
 master/slave.h     |  7 ++---
 3 files changed, 82 insertions(+), 35 deletions(-)

diff --git a/master/fsm_slave.c b/master/fsm_slave.c
index c3010250..c61b888f 100644
--- a/master/fsm_slave.c
+++ b/master/fsm_slave.c
@@ -431,7 +431,7 @@ alloc_eeprom:
         slave->eeprom_size = 0;
         slave->error_flag = 1;
         fsm->state = ec_fsm_slave_state_error;
-        EC_ERR("Failed to allocate EEPROM data on slave %i.\n",
+        EC_ERR("Failed to allocate EEPROM data for slave %u.\n",
                slave->ring_position);
         return;
     }
@@ -453,7 +453,7 @@ alloc_eeprom:
 void ec_fsm_slave_scan_state_eeprom_data(ec_fsm_slave_t *fsm /**< slave state machine */)
 {
     ec_slave_t *slave = fsm->slave;
-    uint16_t *cat_word, cat_type, cat_size;
+    uint16_t *cat_word, cat_type, cat_size, eeprom_word_size = slave->eeprom_size / 2;
 
     if (ec_fsm_sii_exec(&fsm->fsm_sii)) return;
 
@@ -467,7 +467,7 @@ void ec_fsm_slave_scan_state_eeprom_data(ec_fsm_slave_t *fsm /**< slave state ma
 
     // 2 words fetched
 
-    if (fsm->sii_offset + 2 <= slave->eeprom_size / 2) { // 2 words fit
+    if (fsm->sii_offset + 2 <= eeprom_word_size) { // 2 words fit
         memcpy(slave->eeprom_data + fsm->sii_offset * 2,
                fsm->fsm_sii.value, 4);
     }
@@ -476,7 +476,7 @@ void ec_fsm_slave_scan_state_eeprom_data(ec_fsm_slave_t *fsm /**< slave state ma
                fsm->fsm_sii.value, 2);
     }
 
-    if (fsm->sii_offset + 2 < slave->eeprom_size / 2) {
+    if (fsm->sii_offset + 2 < eeprom_word_size) {
         // fetch the next 2 words
         fsm->sii_offset += 2;
         ec_fsm_sii_read(&fsm->fsm_sii, slave, fsm->sii_offset,
@@ -508,39 +508,62 @@ void ec_fsm_slave_scan_state_eeprom_data(ec_fsm_slave_t *fsm /**< slave state ma
     slave->sii_mailbox_protocols =
         EC_READ_U16(slave->eeprom_data + 2 * 0x001C);
 
+    if (eeprom_word_size < EC_FIRST_EEPROM_CATEGORY_OFFSET + 1) {
+        EC_ERR("Unexpected end of EEPROM data in slave %u.\n",
+                slave->ring_position);
+        goto end;
+    }
+
     // evaluate category data
-    cat_word = (uint16_t *) slave->eeprom_data + EC_FIRST_EEPROM_CATEGORY_OFFSET;
+    cat_word =
+        (uint16_t *) slave->eeprom_data + EC_FIRST_EEPROM_CATEGORY_OFFSET;
     while (EC_READ_U16(cat_word) != 0xFFFF) {
+
+        // type and size words must fit
+        if (cat_word + 2 - (uint16_t *) slave->eeprom_data
+                > eeprom_word_size) {
+            EC_ERR("Unexpected end of EEPROM data in slave %u.\n",
+                    slave->ring_position);
+            goto end;
+        }
+
         cat_type = EC_READ_U16(cat_word) & 0x7FFF;
         cat_size = EC_READ_U16(cat_word + 1);
+        cat_word += 2;
+
+        if (cat_word + cat_size - (uint16_t *) slave->eeprom_data
+                > eeprom_word_size) {
+            EC_WARN("Unexpected end of EEPROM data in slave %u.\n",
+                    slave->ring_position);
+            goto end;
+        }
 
         switch (cat_type) {
             case 0x000A:
-                if (ec_slave_fetch_sii_strings(
-                            slave, (uint8_t *) (cat_word + 2)))
+                if (ec_slave_fetch_sii_strings(slave, (uint8_t *) cat_word,
+                            cat_size * 2))
                     goto end;
                 break;
             case 0x001E:
-                ec_slave_fetch_sii_general(
-                        slave, (uint8_t *) (cat_word + 2));
+                if (ec_slave_fetch_sii_general(slave, (uint8_t *) cat_word,
+                            cat_size * 2))
+                    goto end;
                 break;
             case 0x0028:
                 break;
             case 0x0029:
-                if (ec_slave_fetch_sii_syncs(
-                            slave, (uint8_t *) (cat_word + 2), cat_size))
+                if (ec_slave_fetch_sii_syncs(slave, (uint8_t *) cat_word,
+                            cat_size * 2))
                     goto end;
                 break;
             case 0x0032:
-                if (ec_slave_fetch_sii_pdos(
-                            slave, (uint8_t *) (cat_word + 2),
-                            cat_size, EC_TX_PDO))
+                if (ec_slave_fetch_sii_pdos( slave, (uint8_t *) cat_word,
+                            cat_size * 2, EC_TX_PDO))
                     goto end;
                 break;
             case 0x0033:
-                if (ec_slave_fetch_sii_pdos(
-                            slave, (uint8_t *) (cat_word + 2),
-                            cat_size, EC_RX_PDO))
+                if (ec_slave_fetch_sii_pdos( slave, (uint8_t *) cat_word,
+                            cat_size * 2, EC_RX_PDO))
                     goto end;
                 break;
             default:
@@ -549,7 +572,12 @@ void ec_fsm_slave_scan_state_eeprom_data(ec_fsm_slave_t *fsm /**< slave state ma
                             cat_type, slave->ring_position);
         }
 
-        cat_word += cat_size + 2;
+        cat_word += cat_size;
+        if (cat_word - (uint16_t *) slave->eeprom_data >= eeprom_word_size) {
+            EC_WARN("Unexpected end of EEPROM data in slave %u.\n",
+                    slave->ring_position);
+            goto end;
+        }
     }
 
     fsm->state = ec_fsm_slave_state_end;
diff --git a/master/slave.c b/master/slave.c
index f5252ae8..59b6260d 100644
--- a/master/slave.c
+++ b/master/slave.c
@@ -388,12 +388,14 @@ void ec_slave_request_state(ec_slave_t *slave, /**< EtherCAT slave */
 
 /**
    Fetches data from a STRING category.
+   \todo range checking
    \return 0 in case of success, else < 0
 */
 
 int ec_slave_fetch_sii_strings(
         ec_slave_t *slave, /**< EtherCAT slave */
-        const uint8_t *data /**< category data */
+        const uint8_t *data, /**< category data */
+        size_t data_size /**< number of bytes */
         )
 {
     int i;
@@ -444,13 +446,20 @@ out_zero:
    \return 0 in case of success, else < 0
 */
 
-void ec_slave_fetch_sii_general(
+int ec_slave_fetch_sii_general(
         ec_slave_t *slave, /**< EtherCAT slave */
-        const uint8_t *data /**< category data */
+        const uint8_t *data, /**< category data */
+        size_t data_size /**< size in bytes */
         )
 {
     unsigned int i;
 
+    if (data_size != 32) {
+        EC_ERR("Wrong size of general category (%u/32) in slave %u.\n",
+                data_size, slave->ring_position);
+        return -1;
+    }
+
     slave->sii_group = ec_slave_sii_string(slave, data[0]);
     slave->sii_image = ec_slave_sii_string(slave, data[1]);
     slave->sii_order = ec_slave_sii_string(slave, data[2]);
@@ -461,6 +470,8 @@ void ec_slave_fetch_sii_general(
             (data[4] & (0x03 << (i * 2))) >> (i * 2);
 
     slave->sii_current_on_ebus = EC_READ_S16(data + 0x0C);
+
+    return 0;
 }
 
 /*****************************************************************************/
@@ -473,19 +484,26 @@ void ec_slave_fetch_sii_general(
 int ec_slave_fetch_sii_syncs(
         ec_slave_t *slave, /**< EtherCAT slave */
         const uint8_t *data, /**< category data */
-        size_t word_count /**< number of words */
+        size_t data_size /**< number of bytes */
         )
 {
     unsigned int i;
     ec_sync_t *sync;
+    size_t memsize;
 
-    // sync manager struct is 4 words long
-    slave->sii_sync_count = word_count / 4;
+    // one sync manager struct is 4 words long
+    if (data_size % 8) {
+        EC_ERR("Invalid SII sync manager size %u in slave %u.\n",
+                data_size, slave->ring_position);
+        return -1;
+    }
 
-    if (!(slave->sii_syncs =
-                kmalloc(sizeof(ec_sync_t) * slave->sii_sync_count,
-                    GFP_KERNEL))) {
-        EC_ERR("Failed to allocate memory for sync managers.\n");
+    slave->sii_sync_count = data_size / 8;
+
+    memsize = sizeof(ec_sync_t) * slave->sii_sync_count;
+    if (!(slave->sii_syncs = kmalloc(memsize, GFP_KERNEL))) {
+        EC_ERR("Failed to allocate %u bytes for sync managers.\n",
+                memsize);
         slave->sii_sync_count = 0;
         return -1;
     }
@@ -513,7 +531,7 @@ int ec_slave_fetch_sii_syncs(
 int ec_slave_fetch_sii_pdos(
         ec_slave_t *slave, /**< EtherCAT slave */
         const uint8_t *data, /**< category data */
-        size_t word_count, /**< number of words */
+        size_t data_size, /**< number of bytes */
         ec_pdo_type_t pdo_type /**< PDO type */
         )
 {
@@ -521,7 +539,7 @@ int ec_slave_fetch_sii_pdos(
     ec_pdo_entry_t *entry;
     unsigned int entry_count, i;
 
-    while (word_count >= 4) {
+    while (data_size >= 8) {
         if (!(pdo = kmalloc(sizeof(ec_pdo_t), GFP_KERNEL))) {
             EC_ERR("Failed to allocate PDO memory.\n");
             return -1;
@@ -535,7 +553,7 @@ int ec_slave_fetch_sii_pdos(
         pdo->name = ec_slave_sii_string(slave, EC_READ_U8(data + 5));
         list_add_tail(&pdo->list, &slave->sii_pdos);
 
-        word_count -= 4;
+        data_size -= 8;
         data += 8;
 
         for (i = 0; i < entry_count; i++) {
@@ -550,7 +568,7 @@ int ec_slave_fetch_sii_pdos(
             entry->bit_length = EC_READ_U8(data + 5);
             list_add_tail(&entry->list, &pdo->entries);
 
-            word_count -= 4;
+            data_size -= 8;
             data += 8;
         }
 
diff --git a/master/slave.h b/master/slave.h
index 127fc119..392719ff 100644
--- a/master/slave.h
+++ b/master/slave.h
@@ -175,8 +175,9 @@ struct ec_slave
     struct list_head sdo_dictionary; /**< SDO dictionary list */
     struct list_head sdo_confs; /**< list of SDO configurations */
     uint8_t sdo_dictionary_fetched; /**< dictionary has been fetched */
-    uint8_t pdo_mapping_fetched; /**< PDO mapping has been fetched */
     unsigned long jiffies_preop; /**< time, the slave went to PREOP */
+
+    uint8_t pdo_mapping_fetched; /**< PDO mapping has been fetched */
 };
 
 /*****************************************************************************/
@@ -195,8 +196,8 @@ void ec_slave_set_state(ec_slave_t *, ec_slave_state_t);
 void ec_slave_set_online_state(ec_slave_t *, ec_slave_online_state_t);
 
 // SII categories
-int ec_slave_fetch_sii_strings(ec_slave_t *, const uint8_t *);
-void ec_slave_fetch_sii_general(ec_slave_t *, const uint8_t *);
+int ec_slave_fetch_sii_strings(ec_slave_t *, const uint8_t *, size_t);
+int ec_slave_fetch_sii_general(ec_slave_t *, const uint8_t *, size_t);
 int ec_slave_fetch_sii_syncs(ec_slave_t *, const uint8_t *, size_t);
 int ec_slave_fetch_sii_pdos(ec_slave_t *, const uint8_t *, size_t,
         ec_pdo_type_t);
-- 
GitLab