From 7e8fb93206ee95945bb757267832537c13ab4287 Mon Sep 17 00:00:00 2001 From: Cedric Bail Date: Wed, 20 Nov 2013 13:02:37 +0900 Subject: [PATCH] eina: fix a possible race condition during eina_file_close. The lock on the main hash was taken to late (after we took the decision to remove the targeted Eina_File from the cache), this means it was possible to get an Eina_File from the cache that was going to be removed. This patch attempt to fix that potential race condition. Hopefully should fix T461. --- ChangeLog | 4 ++++ NEWS | 3 ++- src/lib/eina/eina_file.c | 9 ++------- src/lib/eina/eina_file_common.c | 15 ++++++++++++--- src/lib/eina/eina_file_common.h | 3 +++ src/lib/eina/eina_file_win32.c | 7 ++----- src/tests/eina/eina_test_file.c | 34 ++++++++++++++++++++++++++++++++++ 7 files changed, 59 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3dc7c28..6c3dea3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2013-11-20 Cedric Bail + + * Eina: Fix a possible race condition during eina_file_close. + 2013-11-19 Tom Hacohen * Evas textblock: Fixed order of tags inserted with markup_app/prepend. diff --git a/NEWS b/NEWS index 6a1f8bd..5ce9bcf 100644 --- a/NEWS +++ b/NEWS @@ -268,7 +268,8 @@ Fixes: - Fix memory leak in eina_xattr_value_ls. - Fix magic failure in eina_value_array_count when array has not been allocated. - Fix issue when wchar_t is signed and eina_unicode does negative array lookups. - - Eina: fix eina_file_map_lines() to not drop of one character in the last line. + - Fix eina_file_map_lines() to not drop of one character in the last line. + - Fix a possible race condition during eina_file_close(). * Eet: - Fix PPC (big endian) image codec bug. - Fix leak in eet_pbkdf2_sha1 with OpenSSL. diff --git a/src/lib/eina/eina_file.c b/src/lib/eina/eina_file.c index 6d7ee56..8ae5a0f 100644 --- a/src/lib/eina/eina_file.c +++ b/src/lib/eina/eina_file.c @@ -305,11 +305,6 @@ eina_file_real_close(Eina_File *file) { Eina_File_Map *map; - if (file->refcount != 0) return; - - eina_hash_free(file->rmap); - eina_hash_free(file->map); - EINA_LIST_FREE(file->dead_map, map) { munmap(map->map, map->length); @@ -320,8 +315,6 @@ eina_file_real_close(Eina_File *file) munmap(file->global_map, file->length); if (file->fd != -1) close(file->fd); - - free(file); } static void @@ -912,6 +905,8 @@ eina_file_open(const char *path, Eina_Bool shared) n->shared = shared; eina_lock_new(&n->lock); eina_hash_direct_add(_eina_file_cache, n->filename, n); + + EINA_MAGIC_SET(n, EINA_FILE_MAGIC); } else { diff --git a/src/lib/eina/eina_file_common.c b/src/lib/eina/eina_file_common.c index 7b05b3b..f5724d7 100644 --- a/src/lib/eina/eina_file_common.c +++ b/src/lib/eina/eina_file_common.c @@ -451,17 +451,26 @@ eina_file_close(Eina_File *file) EINA_SAFETY_ON_NULL_RETURN(file); + eina_lock_take(&_eina_file_lock_cache); + eina_lock_take(&file->lock); file->refcount--; if (file->refcount == 0) leave = EINA_FALSE; eina_lock_release(&file->lock); - if (leave) return; - - eina_lock_take(&_eina_file_lock_cache); + if (leave) goto end; eina_hash_del(_eina_file_cache, file->filename, file); + + // Backend specific file resource close eina_file_real_close(file); + // Generic destruction of the file + eina_hash_free(file->rmap); file->rmap = NULL; + eina_hash_free(file->map); file->map = NULL; + EINA_MAGIC_SET(file, 0); + free(file); + + end: eina_lock_release(&_eina_file_lock_cache); } diff --git a/src/lib/eina/eina_file_common.h b/src/lib/eina/eina_file_common.h index 0ac704d..e3ee0fc 100644 --- a/src/lib/eina/eina_file_common.h +++ b/src/lib/eina/eina_file_common.h @@ -24,11 +24,14 @@ #include "eina_lock.h" #include "eina_list.h" +#define EINA_FILE_MAGIC 0xFEEDBEEF + typedef struct _Eina_File_Map Eina_File_Map; typedef struct _Eina_Lines_Iterator Eina_Lines_Iterator; struct _Eina_File { + EINA_MAGIC; const char *filename; Eina_Hash *map; diff --git a/src/lib/eina/eina_file_win32.c b/src/lib/eina/eina_file_win32.c index cddf2da..8290501 100644 --- a/src/lib/eina/eina_file_win32.c +++ b/src/lib/eina/eina_file_win32.c @@ -366,9 +366,6 @@ eina_file_real_close(Eina_File *file) { Eina_File_Map *map; - eina_hash_free(file->rmap); - eina_hash_free(file->map); - EINA_LIST_FREE(file->dead_map, map) { UnmapViewOfFile(map->map); @@ -380,8 +377,6 @@ eina_file_real_close(Eina_File *file) if (file->fm) CloseHandle(file->fm); if (file->handle) CloseHandle(file->handle); - - free(file); } static void @@ -837,6 +832,8 @@ eina_file_open(const char *path, Eina_Bool shared) n->shared = shared; eina_lock_new(&n->lock); eina_hash_direct_add(_eina_file_cache, n->filename, n); + + EINA_MAGIC_SET(n, EINA_FILE_MAGIC); } else { diff --git a/src/tests/eina/eina_test_file.c b/src/tests/eina/eina_test_file.c index f2f3225..e8f735d 100644 --- a/src/tests/eina/eina_test_file.c +++ b/src/tests/eina/eina_test_file.c @@ -441,6 +441,39 @@ START_TEST(eina_test_file_virtualize) } END_TEST +static void * +_eina_test_file_thread(void *data EINA_UNUSED, Eina_Thread t EINA_UNUSED) +{ + Eina_File *f; + unsigned int i; + + for (i = 0; i < 10000; ++i) + { + f = eina_file_open("/bin/sh", EINA_FALSE); + fail_if(!f); + eina_file_close(f); + } + + return NULL; +} + +START_TEST(eina_test_file_thread) +{ + Eina_Thread th[4]; + unsigned int i; + + fail_if(!eina_init()); + + for (i = 0; i < 4; i++) + fail_if(!(eina_thread_create(&th[i], EINA_THREAD_NORMAL, 0, _eina_test_file_thread, NULL))); + + for (i = 0; i < 4; i++) + fail_if(eina_thread_join(th[i]) != NULL); + + eina_shutdown(); +} +END_TEST + void eina_test_file(TCase *tc) { @@ -449,5 +482,6 @@ eina_test_file(TCase *tc) tcase_add_test(tc, eina_file_ls_simple); tcase_add_test(tc, eina_file_map_new_test); tcase_add_test(tc, eina_test_file_virtualize); + tcase_add_test(tc, eina_test_file_thread); } -- 2.7.4