Open_Plugin add checksum on struct offsets

Adding a checksum over the struct offset will allow checking
for compatibility across machines rather than using packed structs
to ensure compability

For any file created by the user from the device  this isn't really a concern

But for files between machines, across installs (sim v device),
possibly even across compilers this at least will alert the user
rather than returning junk data

Change-Id: Id0531bbaa7013dce24dece270849f0a10ac99c20

3 files changed, 73 insertions, 24 deletions

diff --git a/apps/open_plugin.c b/apps/open_plugin.c
index 32b5195334..fe18071454 100644
--- a/apps/open_plugin.c
+++ b/apps/open_plugin.c
@@ -38,6 +38,8 @@
 
 struct open_plugin_entry_t open_plugin_entry = {0};
 
+static const uint32_t open_plugin_csum = OPEN_PLUGIN_CHECKSUM;
+
 static const int op_entry_sz = sizeof(struct open_plugin_entry_t);
 
 static int open_plugin_hash_get_entry(uint32_t hash,
@@ -60,6 +62,13 @@ static inline void op_clear_entry(struct open_plugin_entry_t *entry)
     entry->lang_id = -1;
 }
 
+static int op_entry_checksum(struct open_plugin_entry_t *entry)
+{
+    if (!entry || entry->checksum != open_plugin_csum)
+        return 0;
+    return 1;
+}
+
 static int op_find_entry(int fd, struct open_plugin_entry_t *entry,
                                     uint32_t hash, int32_t lang_id)
 {
@@ -75,6 +84,13 @@ static int op_find_entry(int fd, struct open_plugin_entry_t *entry,
                (entry->hash == hash && hash != 0) ||
                (lang_id == OPEN_PLUGIN_LANG_IGNOREALL))/* return first entry found */
             {
+                /* sanity check */
+                if (op_entry_checksum(entry) <= 0)
+                {
+                    splashf(HZ * 2, "OpenPlugin Invalid entry");
+                    ret = OPEN_PLUGIN_NOT_FOUND;
+                    break;
+                }
                 /* NULL terminate fields NOTE -- all are actually +1 larger */
                 entry->name[OPEN_PLUGIN_NAMESZ] = '\0';
                 /*entry->key[OPEN_PLUGIN_BUFSZ] = '\0';*/
@@ -111,24 +127,27 @@ static int op_update_dat(struct open_plugin_entry_t *entry, bool clear)
     logf("OP update hash: %x lang_id: %d", hash, lang_id);
     logf("OP update name: %s clear: %d", entry->name, (int) clear);
     logf("OP update %s %s %s", entry->name, entry->path, entry->param);
+
 #if (CONFIG_STORAGE & STORAGE_ATA) /* Harddrive -- update existing */
     logf("OP update *Updating entries* %s", OPEN_PLUGIN_DAT);
     fd = open(OPEN_PLUGIN_DAT, O_RDWR | O_CREAT, 0666);
 
     if (fd < 0)
         return OPEN_PLUGIN_NOT_FOUND;
-    /* Only read the hash and lang id */
-    uint32_t hash_langid[2] = {0};
-    while (read(fd, &hash_langid, sizeof(hash_langid)) == sizeof(hash_langid))
+    /* Only read the hash lang id and checksum */
+    uint32_t hash_langid_csum[3] = {0};
+    const off_t hlc_sz = sizeof(hash_langid_csum);
+    while (read(fd, &hash_langid_csum, hlc_sz) == hlc_sz)
     {
-        if (hash_langid[0] == hash || (int32_t)hash_langid[1] == lang_id)
+        if ((hash_langid_csum[0] == hash || (int32_t)hash_langid_csum[1] == lang_id) &&
+             hash_langid_csum[2] == open_plugin_csum)
         {
             logf("OP update *Entry Exists* hash: %x langid: %d",
-                hash_langid[0], (int32_t)hash_langid[1]);
-            lseek(fd, 0-sizeof(hash_langid), SEEK_CUR);/* back to the start of record */
+                hash_langid_csum[0], (int32_t)hash_langid[1]);
+            lseek(fd, 0-hlc_sz, SEEK_CUR);/* back to the start of record */
             break;
         }
-        lseek(fd, op_entry_sz - sizeof(hash_langid), SEEK_CUR); /* finish record */
+        lseek(fd, op_entry_sz - hlc_sz, SEEK_CUR); /* finish record */
     }
     write(fd, entry, op_entry_sz);
     close(fd);
@@ -146,7 +165,8 @@ static int op_update_dat(struct open_plugin_entry_t *entry, bool clear)
         /* copy non-duplicate entries back from original */
         while (read(fd1, entry, op_entry_sz) == op_entry_sz)
         {
-            if (entry->hash != hash && entry->lang_id != lang_id)
+            if (entry->hash != hash && entry->lang_id != lang_id &&
+               op_entry_checksum(entry) > 0)
             {
                 write(fd, entry, op_entry_sz);
             }
@@ -202,8 +222,9 @@ uint32_t open_plugin_add_path(const char *key, const char *plugin, const char *p
 
     if (plugin)
     {
-        open_plugin_entry.hash = hash;
-        open_plugin_entry.lang_id = lang_id;
+        open_plugin_entry.hash     = hash;
+        open_plugin_entry.lang_id  = lang_id;
+        open_plugin_entry.checksum = open_plugin_csum;
         /* name */
         if (path_basename(plugin, (const char **)&pos) == 0)
             pos = "\0";
@@ -242,7 +263,7 @@ retnhash:
 
 void open_plugin_browse(const char *key)
 {
-    logf("OP Browse");
+    logf("OP browse");
     struct browse_context browse;
     char tmp_buf[OPEN_PLUGIN_BUFSZ+1];
     open_plugin_get_entry(key, &open_plugin_entry);
diff --git a/apps/open_plugin.h b/apps/open_plugin.h
index e1d49bf329..adfb9a75bc 100644
--- a/apps/open_plugin.h
+++ b/apps/open_plugin.h
@@ -41,19 +41,31 @@ enum {
     OPEN_PLUGIN_LANG_IGNORE     = (-2),
     OPEN_PLUGIN_LANG_IGNOREALL  = (-3),
     OPEN_PLUGIN_NOT_FOUND     = (-1),
-    OPEN_PLUGIN_NEEDS_FLUSHED = (-2)
+    OPEN_PLUGIN_NEEDS_FLUSHED = (-2),
 };
 
 struct open_plugin_entry_t
 {
-/* hash and lang_id need to be the first items */
+/* hash lang_id checksum need to be the first items */
     uint32_t hash;
     int32_t  lang_id;
+    uint32_t  checksum;
     char name[OPEN_PLUGIN_NAMESZ+1];
     /*char key[OPEN_PLUGIN_BUFSZ+1];*/
     char path[OPEN_PLUGIN_BUFSZ+1];
     char param[OPEN_PLUGIN_BUFSZ+1];
-}__attribute__((packed));
+};
+
+#define OPEN_PLUGIN_CHECKSUM (uint32_t)              \
+(                                                    \
+    (sizeof(struct open_plugin_entry_t) << 16)     + \
+    offsetof(struct open_plugin_entry_t, hash)     + \
+    offsetof(struct open_plugin_entry_t, lang_id)  + \
+    offsetof(struct open_plugin_entry_t, checksum) + \
+    offsetof(struct open_plugin_entry_t, name)     + \
+    /*offsetof(struct open_plugin_entry_t, key)+*/   \
+    offsetof(struct open_plugin_entry_t, path)     + \
+    offsetof(struct open_plugin_entry_t, param))
 
 inline static void open_plugin_get_hash(const char *key, uint32_t *hash)
 {
diff --git a/apps/plugins/open_plugins.c b/apps/plugins/open_plugins.c
index b8d11d2ae5..3a0c34d8d6 100644
--- a/apps/plugins/open_plugins.c
+++ b/apps/plugins/open_plugins.c
@@ -49,7 +49,8 @@
 static int fd_dat;
 static struct gui_synclist lists;
 struct open_plugin_entry_t op_entry;
-const off_t op_entry_sz = sizeof(struct open_plugin_entry_t);
+static const uint32_t open_plugin_csum = OPEN_PLUGIN_CHECKSUM;
+static const off_t op_entry_sz = sizeof(struct open_plugin_entry_t);
 
 /* we only need the names for the first menu so don't bother reading paths yet */
 const off_t op_name_sz = OPEN_PLUGIN_NAMESZ + (op_entry.name - (char*)&op_entry);
@@ -101,6 +102,15 @@ static bool op_entry_read_name(int fd, int selected_item)
     return op_entry_read(fd, selected_item, op_name_sz);
 }
 
+static int op_entry_checksum(void)
+{
+    if (op_entry.checksum != open_plugin_csum)
+    {
+        return 0;
+    }
+    return 1;
+}
+
 static int op_entry_read_opx(const char *path)
 {
     int ret = -1;
@@ -112,13 +122,14 @@ static int op_entry_read_opx(const char *path)
     if(len > OP_LEN && rb->strcasecmp(&((path)[len-OP_LEN]), "." OP_EXT) == 0)
     {
         fd_opx = rb->open(path, O_RDONLY);
-        if (fd_opx)
+        if (fd_opx >= 0)
         {
             filesize = rb->filesize(fd_opx);
             ret = filesize;
             if (filesize == op_entry_sz && !op_entry_read(fd_opx, 0, op_entry_sz))
                 ret = 0;
-
+            else if (op_entry_checksum() <= 0)
+                ret = 0;
             rb->close(fd_opx);
         }
     }
@@ -131,7 +142,7 @@ static void op_entry_export(int selection)
     int fd = -1;
     char filename [MAX_PATH + 1];
 
-    if (!op_entry_read(fd_dat, selection, op_entry_sz))
+    if (!op_entry_read(fd_dat, selection, op_entry_sz) || op_entry_checksum() <= 0)
         goto failure;
 
     rb->snprintf(filename, MAX_PATH, "%s/%s", PLUGIN_APPS_DIR, op_entry.name);
@@ -161,6 +172,11 @@ failure:
 
 }
 
+static void op_entry_set_checksum(void)
+{
+        op_entry.checksum = open_plugin_csum;
+}
+
 static void op_entry_set_name(void)
 {
     char tmp_buf[OPEN_PLUGIN_NAMESZ+1];
@@ -277,12 +293,12 @@ static int op_entry_transfer(int fd, int fd_tmp,
                             void *data)
 {
     int entries = -1;
-    if (fd_tmp && fd && rb->lseek(fd, 0, SEEK_SET) == 0)
+    if (fd_tmp >= 0 && fd >= 0 && rb->lseek(fd, 0, SEEK_SET) == 0)
     {
         entries = 0;
         while (rb->read(fd, &op_entry, op_entry_sz) == op_entry_sz)
         {
-            if (compfn && compfn(&op_entry, entries, data) > 0)
+            if (compfn && compfn(&op_entry, entries, data) > 0 && op_entry_checksum() > 0)
             {
                 rb->write(fd_tmp, &op_entry, op_entry_sz);
                 entries++;
@@ -359,7 +375,7 @@ static uint32_t op_entry_add_path(const char *key, const char *plugin, const cha
                     op_entry.hash = newhash;
                 }
             }
-
+            op_entry_set_checksum();
             rb->write(fd_tmp, &op_entry, op_entry_sz); /* add new entry first */
         }
         else if(op_entry_read_opx(plugin) == op_entry_sz)
@@ -374,13 +390,13 @@ static uint32_t op_entry_add_path(const char *key, const char *plugin, const cha
                 open_plugin_get_hash(op_entry.path, &hash);
 
             op_entry.hash = hash;
-
+            op_entry_set_checksum();
             rb->write(fd_tmp, &op_entry, op_entry_sz); /* add new entry first */
         }
         else
         {
             if (op_entry.lang_id != LANG_SHORTCUTS)
-                rb->splashf(HZ / 2, rb->str(LANG_OPEN_PLUGIN_NOT_A_PLUGIN), pos);
+                rb->splashf(HZ * 2, rb->str(LANG_OPEN_PLUGIN_NOT_A_PLUGIN), pos);
             return 0;
         }
     }
@@ -845,7 +861,7 @@ reopen_datfile:
         synclist_set(MENU_ID_MAIN, selection, items, 1);
         rb->gui_synclist_draw(&lists);
 
-        while (!exit)
+        while (!exit && fd_dat >= 0)
         {
             action = rb->get_action(CONTEXT_LIST,TIMEOUT_BLOCK);