jpeg pushed a commit to branch master.
http://git.enlightenment.org/core/efl.git/commit/?id=05c051405e89ef595f117f637dbbb0104262c218
commit 05c051405e89ef595f117f637dbbb0104262c218
Author: Jean-Philippe Andre <jp.an...@samsung.com>
Date: Fri Sep 15 15:56:17 2017 +0900
eina file: Ensure populate is safe to call
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
---
src/lib/eina/eina_file.c | 60 ++++++++++++++++++++++++++++-------------
src/tests/eina/eina_test_file.c | 12 ++++++---
2 files changed, 51 insertions(+), 21 deletions(-)
diff --git a/src/lib/eina/eina_file.c b/src/lib/eina/eina_file.c
index 97dec3e952..e4582697c1 100644
--- 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;
@@ -331,11 +334,24 @@ _eina_file_map_populate(char *map, unsigned long 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)
{
@@ -354,6 +370,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
@@ -914,7 +945,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);
@@ -941,7 +972,7 @@ eina_file_map_all(Eina_File *file, Eina_File_Populate rule)
else
hugetlb = file->global_hugetlb;
- _eina_file_map_rule_apply(rule, file->global_map, file->length,
hugetlb);
+ _eina_file_map_rule_apply(rule, file->global_map, 0, file->length,
file->length, hugetlb);
file->global_refcount++;
ret = file->global_map;
}
@@ -986,7 +1017,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));
@@ -1016,7 +1047,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);
@@ -1064,21 +1095,14 @@ EAPI void
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);
if (map == file->global_map)
- {
- _eina_file_map_rule_apply(rule, ((char*) map) + offset, length,
- file->global_hugetlb);
- }
- else
- {
- Eina_File_Map *em;
-
- em = eina_hash_find(file->rmap, &map);
- if (em) _eina_file_map_rule_apply(rule, ((char *) em->map) + offset,
- length, em->hugetlb);
- }
+ _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/tests/eina/eina_test_file.c b/src/tests/eina/eina_test_file.c
index 28f0a3b527..07b42daf2e 100644
--- a/src/tests/eina/eina_test_file.c
+++ b/src/tests/eina/eina_test_file.c
@@ -405,9 +405,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;
@@ -417,7 +415,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);
--
