From 9528d1bf64cd8aefd970023af93eb0b638af7a1c Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 18 Apr 2019 17:53:59 -0400 Subject: [PATCH 1/3] Add secure alloc/free to posix env Signed-off-by: Adam Rutkowski --- env/posix/ocf_env.h | 63 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/env/posix/ocf_env.h b/env/posix/ocf_env.h index 7c0ff53..603750a 100644 --- a/env/posix/ocf_env.h +++ b/env/posix/ocf_env.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #include "ocf_env_list.h" @@ -59,6 +60,15 @@ typedef uint64_t sector_t; #define PAGE_SIZE 4096 +/* *** DEBUGING *** */ + +#define ENV_WARN(cond, fmt...) printf(fmt) +#define ENV_WARN_ON(cond) ; +#define ENV_WARN_ONCE(cond, fmt...) ENV_WARN(cond, fmt) + +#define ENV_BUG() assert(0) +#define ENV_BUG_ON(cond) assert(!(cond)) + /* *** MEMORY MANAGEMENT *** */ #define ENV_MEM_NORMAL 0 #define ENV_MEM_NOIO 0 @@ -99,6 +109,49 @@ static inline void env_vfree(const void *ptr) free((void *)ptr); } + +/* *** SECURE MEMORY MANAGEMENT *** */ + +/* + * OCF adapter can opt to take additional steps to securely allocate and free + * memory used by OCF to store cache metadata. This is to prevent other + * entities in the system from acquiring parts of OCF cache metadata via + * memory allocations. If this is not a concern in given product, secure + * alloc/free should default to vmalloc/vfree. + * + * Memory returned from secure alloc is not expected to be physically continous + * nor zeroed. + */ + +/* default to standard memory allocations for secure allocations */ +#define SECURE_MEMORY_HANDLING 0 + +static inline void *env_secure_alloc(size_t size) +{ + void *ptr = malloc(size); + +#if SECURE_MEMORY_HANDLING + if (ptr && mlock(ptr, size)) { + free(ptr); + ptr = NULL; + } +#endif + + return ptr; +} + +static inline void env_secure_free(const void *ptr, size_t size) +{ + if (ptr) { +#if SECURE_MEMORY_HANDLING + memset(ptr, size, 0); + /* TODO: flush CPU caches ? */ + ENV_BUG_ON(munlock(ptr)); +#endif + free((void*)ptr); + } +} + static inline uint64_t env_get_free_memory(void) { return sysconf(_SC_PAGESIZE) * sysconf(_SC_AVPHYS_PAGES); @@ -613,21 +666,15 @@ static inline void env_sort(void *base, size_t num, size_t size, strncpy(dest, src, min(dmax, slen)); \ 0; \ }) -/* *** DEBUGING *** */ - -#define ENV_WARN(cond, fmt...) printf(fmt) -#define ENV_WARN_ON(cond) ; -#define ENV_WARN_ONCE(cond, fmt...) ENV_WARN(cond, fmt) - -#define ENV_BUG() assert(0) -#define ENV_BUG_ON(cond) assert(!(cond)) +/* *** MISC UTILITIES *** */ #define container_of(ptr, type, member) ({ \ const typeof(((type *)0)->member)*__mptr = (ptr); \ (type *)((char *)__mptr - offsetof(type, member)); }) #define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x))) +/* *** TIME *** */ static inline void env_msleep(uint64_t n) { usleep(n * 1000); From c5a80cc4887d0f387b7889c40e3abcd985ed1eff Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 18 Apr 2019 14:45:51 -0400 Subject: [PATCH 2/3] Use env_secure_(alloc/free) macro for metadata allocations Adapter can opt to take additional steps to securely allocate memory used by OCF to store cache metadata. Typically this would involve mlocking pages and zeroing memory before deallocation. Memory allocated using secure_alloc is not expected to be zeroed or physically continous. Signed-off-by: Adam Rutkowski --- src/metadata/metadata_hash.c | 5 +++-- src/metadata/metadata_raw.c | 5 +++-- src/metadata/metadata_raw_dynamic.c | 6 +++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/metadata/metadata_hash.c b/src/metadata/metadata_hash.c index a3b809c..85946a3 100644 --- a/src/metadata/metadata_hash.c +++ b/src/metadata/metadata_hash.c @@ -680,7 +680,7 @@ exit: ctx_data_free(ctx, context->data.core_config.data); ctx_data_free(ctx, context->data.superblock.data); - env_vfree(context); + env_secure_free(context, sizeof(*context)); } static void ocf_metadata_query_cores_end_io(struct ocf_io *io, int error) @@ -800,11 +800,12 @@ void ocf_metadata_hash_query_cores(ocf_ctx_t owner, ocf_volume_t volume, } /* intialize query context */ - context = env_vzalloc(sizeof(*context)); + context = env_secure_alloc(sizeof(*context)); if (!context) { cmpl(priv, -ENOMEM, 0); return; } + ENV_BUG_ON(env_memset(context, sizeof(*context), 0)); context->ctx = owner; context->params.cmpl = cmpl; context->params.priv = priv; diff --git a/src/metadata/metadata_raw.c b/src/metadata/metadata_raw.c index 4f8941f..cee7eeb 100644 --- a/src/metadata/metadata_raw.c +++ b/src/metadata/metadata_raw.c @@ -78,7 +78,7 @@ static int _raw_ram_deinit(ocf_cache_t cache, OCF_DEBUG_TRACE(cache); if (raw->mem_pool) { - env_vfree(raw->mem_pool); + env_secure_free(raw->mem_pool, raw->mem_pool_limit); raw->mem_pool = NULL; } @@ -99,9 +99,10 @@ static int _raw_ram_init(ocf_cache_t cache, mem_pool_size = raw->ssd_pages; mem_pool_size *= PAGE_SIZE; raw->mem_pool_limit = mem_pool_size; - raw->mem_pool = env_vzalloc(mem_pool_size); + raw->mem_pool = env_secure_alloc(mem_pool_size); if (!raw->mem_pool) return -ENOMEM; + ENV_BUG_ON(env_memset(raw->mem_pool, mem_pool_size, 0)); return 0; } diff --git a/src/metadata/metadata_raw_dynamic.c b/src/metadata/metadata_raw_dynamic.c index 35031bd..3f9cd54 100644 --- a/src/metadata/metadata_raw_dynamic.c +++ b/src/metadata/metadata_raw_dynamic.c @@ -125,7 +125,7 @@ int raw_dynamic_deinit(ocf_cache_t cache, OCF_DEBUG_TRACE(cache); for (i = 0; i < raw->ssd_pages; i++) - env_free(ctrl->pages[i]); + env_secure_free(ctrl->pages[i], PAGE_SIZE); env_vfree(ctrl); raw->priv = NULL; @@ -296,7 +296,7 @@ static void raw_dynamic_load_all_complete( context->cmpl(context->priv, error); ocf_req_put(context->req); - env_free(context->page); + env_secure_free(context->page, PAGE_SIZE); env_free(context->zpage); ctx_data_free(context->cache->owner, context->data); env_vfree(context); @@ -383,7 +383,7 @@ static int raw_dynamic_load_all_update(struct ocf_request *req) for (i_page = 0; i_page < count; i_page++, context->i++) { if (!context->page) { - context->page = env_malloc(PAGE_SIZE, ENV_MEM_NORMAL); + context->page = env_secure_alloc(PAGE_SIZE); if (!context->page) { /* Allocation error */ result = -OCF_ERR_NO_MEM; From cf24b46a58d1dbe7026befcf44916c4fe1988714 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 18 Apr 2019 15:56:52 -0400 Subject: [PATCH 3/3] posix env: evaluate ENV_BUG_ON condition unconditionally OCF depends on ENV_BUG_ON condition to be evaluated as it may have side effects. So simple implementation with "assert(!cond)" is not good enough as it will likely be noop in release build. Signed-off-by: Adam Rutkowski --- env/posix/ocf_env.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/env/posix/ocf_env.h b/env/posix/ocf_env.h index 603750a..83d0758 100644 --- a/env/posix/ocf_env.h +++ b/env/posix/ocf_env.h @@ -67,7 +67,7 @@ typedef uint64_t sector_t; #define ENV_WARN_ONCE(cond, fmt...) ENV_WARN(cond, fmt) #define ENV_BUG() assert(0) -#define ENV_BUG_ON(cond) assert(!(cond)) +#define ENV_BUG_ON(cond) do { if (cond) ENV_BUG(); } while (0) /* *** MEMORY MANAGEMENT *** */ #define ENV_MEM_NORMAL 0