eina_file: Ensure populate is safe to call

Fix map_populate on the global map

This patch includes two commits

1) [PATCH] eina file: Ensure populate is safe to call
   (commit 05c051405e89ef595f117f637dbbb0104262c218)
 This makes sure that the call to madvise is safe. On Linux it's not too
 much of an issue as checks are made inside madvise, and the worst that
 can happen is an error is returned (EINVAL). Not great.

 But if MAP_POPULATE is not present, as is the case on *BSD, then the
 internal function _eina_file_map_populate() is used for the populate
 rule. In that case actual data is read and we should make sure not to
 trigger a segfault or bus error.

 Also, this makes sure that in case of HugeTLB we actually populate all
 pages, rather than one page out of 8 (we were jumping by 16Mb instead of
 2Mb).

 Note: Can we get the size of a HugeTLB at runtime? We're assuming 2Mb
 which might very well not be the case!
 See: https://wiki.debian.org/Hugepages

 Tested by disabling MAP_POPULATE and observing crashes :)

 @fix

2) [PATCH] eina file: Fix map_populate on the global map
   (commit b2ea60691e45674a4e283e0535634bdd699544ea)
 If eina_file_map_all() is called, the map isn't added to the internal
 hash "rmap" and so _eina_file_map_rule_apply() would never be called.

 @fix

Change-Id: Ib074aff31e3498fb7fca7c5ca792d5bdf8e8bd17

diff --git a/src/lib/eina/eina_file.c b/src/lib/eina/eina_file.c
index 08e3a2d..94c2b30 100644 (file)
--- a/src/lib/eina/eina_file.c
+++ b/src/lib/eina/eina_file.c
@@ -68,7 +68,10 @@
  */
 
 #define EINA_SMALL_PAGE eina_cpu_page_size()
-#define EINA_HUGE_PAGE 16 * 1024 * 1024
+
+// FIXME: This assumes HugeTLB size of 2Mb. How to get this information at runtime?
+#define EINA_HUGE_PAGE (2 * 1024 * 1024)
+#define EINA_HUGE_PAGE_MIN (8 * EINA_HUGE_PAGE)
 
 #ifdef HAVE_DIRENT_H
 typedef struct _Eina_File_Iterator Eina_File_Iterator;
@@ -344,11 +347,24 @@ _eina_file_map_populate(char *map, unsigned int size, Eina_Bool hugetlb)
 }
 #endif
 
+static char *
+_page_aligned_address(const char *map, unsigned long int offset, Eina_Bool hugetlb)
+{
+   const uintptr_t align = hugetlb ? EINA_HUGE_PAGE : EINA_SMALL_PAGE;
+   uintptr_t pmap = (uintptr_t) map;
+
+   pmap = (pmap + offset) - ((pmap + offset) & (align - 1));
+
+   return (char *) pmap;
+}
+
 static int
-_eina_file_map_rule_apply(Eina_File_Populate rule, void *addr, unsigned long int size, Eina_Bool hugetlb)
+_eina_file_map_rule_apply(Eina_File_Populate rule, const void *map, unsigned long int offset,
+                          unsigned long int size, unsigned long long maplen, Eina_Bool hugetlb)
 {
    int tmp = 42;
    int flag = MADV_RANDOM;
+   char *addr;
 
    switch (rule)
      {
@@ -367,6 +383,21 @@ _eina_file_map_rule_apply(Eina_File_Populate rule, void *addr, unsigned long int
       default: return tmp; break;
      }
 
+   if (offset >= maplen) return tmp;
+
+   // Align address, clamp size
+   addr = _page_aligned_address(map, offset, hugetlb);
+   if (size > 0)
+     {
+        size += ((char *) map + offset) - addr;
+        offset -= ((char *) map + offset) - addr;
+        if ((offset + size) > maplen)
+          {
+             if (offset > maplen) return tmp;
+             size = maplen - offset;
+          }
+     }
+
    madvise(addr, size, flag);
 
 #ifndef MAP_POPULATE
@@ -928,7 +959,7 @@ eina_file_map_all(Eina_File *file, Eina_File_Populate rule)
    if (rule == EINA_FILE_POPULATE) flags |= MAP_POPULATE;
 #endif
 #ifdef MAP_HUGETLB
-   if (file->length > EINA_HUGE_PAGE) flags |= MAP_HUGETLB;
+   if (file->length >= EINA_HUGE_PAGE_MIN) flags |= MAP_HUGETLB;
 #endif
 
    eina_mmap_safety_enabled_set(EINA_TRUE);
@@ -950,7 +981,12 @@ eina_file_map_all(Eina_File *file, Eina_File_Populate rule)
 #ifdef MAP_HUGETLB
         hugetlb = !!(flags & MAP_HUGETLB);
 #endif
-        _eina_file_map_rule_apply(rule, file->global_map, file->length, hugetlb);
+        if (!file->global_refcount)
+          file->global_hugetlb = hugetlb;
+        else
+          hugetlb = file->global_hugetlb;
+        _eina_file_map_rule_apply(rule, file->global_map, 0, file->length, file->length, hugetlb);
+
         file->global_refcount++;
         ret = file->global_map;
      }
@@ -995,7 +1031,7 @@ eina_file_map_new(Eina_File *file, Eina_File_Populate rule,
         if (rule == EINA_FILE_POPULATE) flags |= MAP_POPULATE;
 #endif
 #ifdef MAP_HUGETLB
-        if (length > EINA_HUGE_PAGE) flags |= MAP_HUGETLB;
+        if (length > EINA_HUGE_PAGE_MIN) flags |= MAP_HUGETLB;
 #endif
 
         map = malloc(sizeof (Eina_File_Map));
@@ -1025,7 +1061,7 @@ eina_file_map_new(Eina_File *file, Eina_File_Populate rule,
 
    map->refcount++;
 
-   _eina_file_map_rule_apply(rule, map->map, length, map->hugetlb);
+   _eina_file_map_rule_apply(rule, map->map, 0, length, map->length, map->hugetlb);
 
    eina_lock_release(&file->lock);
 
@@ -1074,12 +1110,13 @@ eina_file_map_populate(Eina_File *file, Eina_File_Populate rule, const void *map
                        unsigned long int offset, unsigned long int length)
 {
    Eina_File_Map *em;
-   
+
    EINA_SAFETY_ON_NULL_RETURN(file);
    eina_lock_take(&file->lock);
-   em = eina_hash_find(file->rmap, &map);
-   if (em) _eina_file_map_rule_apply(rule, ((char *)em->map) + offset,
-                                     length, em->hugetlb);
+   if (map == file->global_map)
+     _eina_file_map_rule_apply(rule, map, offset, length, file->length, file->global_hugetlb);
+   else if ((em = eina_hash_find(file->rmap, &map)) != NULL)
+     _eina_file_map_rule_apply(rule, map, offset, length, em->length, em->hugetlb);
    eina_lock_release(&file->lock);
 }
 

diff --git a/src/lib/eina/eina_file_common.h b/src/lib/eina/eina_file_common.h
index e8241d9..7216efb 100644 (file)
--- a/src/lib/eina/eina_file_common.h
+++ b/src/lib/eina/eina_file_common.h
@@ -102,6 +102,7 @@ struct _Eina_File
    Eina_Bool shared : 1;         /**< Indicates whether this file can be shared */
    Eina_Bool delete_me : 1;      /**< Indicates that this file should be deleted */
    Eina_Bool global_faulty : 1;  /**< Indicates whether #global_map is bad */
+   Eina_Bool global_hugetlb : 1; /**< Indicates whether #global_map uses HugeTLB */
    Eina_Bool virtual : 1;        /**< Indicates that this is a virtual file */
 };
 

diff --git a/src/tests/eina/eina_test_file.c b/src/tests/eina/eina_test_file.c
index db586eb..eae6105 100644 (file)
--- a/src/tests/eina/eina_test_file.c
+++ b/src/tests/eina/eina_test_file.c
@@ -398,9 +398,7 @@ START_TEST(eina_file_map_new_test)
    file_map = eina_file_map_new(e_file, EINA_FILE_WILLNEED, map_offset, map_length); 
    fail_if(!file_map);
    correct_map_check= strcmp((char*) file_map, eina_map_test_string); 
-   fail_if(correct_map_check != 0);  
-
-   eina_file_map_free(e_file, file_map);
+   fail_if(correct_map_check != 0);
 
    // test : offset = memory_page_size AND length = file->length - memory_page_size => correct partly map
    map_offset = memory_page_size;
@@ -410,7 +408,15 @@ START_TEST(eina_file_map_new_test)
    correct_map_check = strcmp((char*)file2_map, big_buffer + memory_page_size); 
    fail_if(correct_map_check != 0);  
 
+   // test no crash with eina_file_map_populate()
+   eina_file_map_populate(e_file, EINA_FILE_POPULATE, file_map, 0, 0);
+   eina_file_map_populate(e_file, EINA_FILE_POPULATE, file_map, file_length / 2, 0);
+   eina_file_map_populate(e_file, EINA_FILE_POPULATE, file_map, 0, file_length * 2);
+   eina_file_map_populate(e_file, EINA_FILE_POPULATE, file_map, file_length / 2, big_buffer_size);
+   eina_file_map_populate(e_file, EINA_FILE_POPULATE, file_map, big_buffer_size + 1, file_length);
+
    eina_file_map_free(e_file, file_map);
+   eina_file_map_free(e_file, file_map); // test no crash
    eina_file_map_free(e_file2, file2_map);
    eina_file_close(e_file);
    eina_file_close(e_file2);