Skip to content

Commit

Permalink
core: scrub user-tainted kernel heap memory before freeing it
Browse files Browse the repository at this point in the history
Some syscalls can be used to poison kernel heap memory. Data copied from
userland is not wiped when the syscall returns. For instance, when doing
syscall_log() one can copy arbitrary data of variable length onto kernel
memory. When free() is called, the block is returned to the memory pool,
tainted with that userland data. This might be used in combination with
some other vulnerability to produce an exploit.

This patch uses free_wipe() to clear the buffers that have been used to
store user-provided data before returning them to the heap.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [1.4]
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>
Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
  • Loading branch information
jforissier committed May 13, 2019
1 parent 4e57065 commit 70b6131
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 24 deletions.
38 changes: 19 additions & 19 deletions core/tee/tee_svc.c
Expand Up @@ -2,25 +2,25 @@
/*
* Copyright (c) 2014, STMicroelectronics International N.V.
*/
#include <util.h>
#include <kernel/tee_common_otp.h>
#include <kernel/chip_services.h>
#include <kernel/pseudo_ta.h>
#include <kernel/tee_common.h>
#include <tee_api_types.h>
#include <kernel/tee_common_otp.h>
#include <kernel/tee_ta_manager.h>
#include <utee_types.h>
#include <tee/tee_svc.h>
#include <tee/tee_cryp_utl.h>
#include <mm/tee_mmu.h>
#include <mm/tee_mm.h>
#include <mm/core_memprot.h>
#include <kernel/tee_time.h>

#include <user_ta_header.h>
#include <trace.h>
#include <kernel/trace_ta.h>
#include <kernel/chip_services.h>
#include <kernel/pseudo_ta.h>
#include <mm/core_memprot.h>
#include <mm/mobj.h>
#include <mm/tee_mm.h>
#include <mm/tee_mmu.h>
#include <stdlib_ext.h>
#include <tee_api_types.h>
#include <tee/tee_cryp_utl.h>
#include <tee/tee_svc.h>
#include <trace.h>
#include <user_ta_header.h>
#include <utee_types.h>
#include <util.h>

vaddr_t tee_svc_uref_base;

Expand All @@ -41,7 +41,7 @@ void syscall_log(const void *buf __maybe_unused, size_t len __maybe_unused)
trace_ext_puts(kbuf);
}

free(kbuf);
free_wipe(kbuf);
#endif
}

Expand Down Expand Up @@ -490,7 +490,7 @@ TEE_Result syscall_get_property_name_to_index(unsigned long prop_set,
}

out:
free(kname);
free_wipe(kname);
return res;
}

Expand Down Expand Up @@ -793,9 +793,9 @@ TEE_Result syscall_open_ta_session(const TEE_UUID *dest,
tee_svc_copy_to_user(ret_orig, &ret_o, sizeof(ret_o));

out_free_only:
free(param);
free(uuid);
free(clnt_id);
free_wipe(param);
free_wipe(uuid);
free_wipe(clnt_id);
return res;
}

Expand Down
11 changes: 6 additions & 5 deletions core/tee/tee_svc_cryp.c
Expand Up @@ -8,6 +8,7 @@
#include <crypto/crypto.h>
#include <kernel/tee_ta_manager.h>
#include <mm/tee_mmu.h>
#include <stdlib_ext.h>
#include <string_ext.h>
#include <string.h>
#include <sys/queue.h>
Expand Down Expand Up @@ -1597,7 +1598,7 @@ TEE_Result syscall_cryp_obj_populate(unsigned long obj,
o->info.handleFlags |= TEE_HANDLE_FLAG_INITIALIZED;

out:
free(attrs);
free_wipe(attrs);
return res;
}

Expand Down Expand Up @@ -1874,7 +1875,7 @@ TEE_Result syscall_obj_generate_key(unsigned long obj, unsigned long key_size,
}

out:
free(params);
free_wipe(params);
if (res == TEE_SUCCESS) {
o->info.keySize = key_size;
o->info.handleFlags |= TEE_HANDLE_FLAG_INITIALIZED;
Expand Down Expand Up @@ -2916,7 +2917,7 @@ TEE_Result syscall_cryp_derive_key(unsigned long state,
res = TEE_ERROR_NOT_SUPPORTED;

out:
free(params);
free_wipe(params);
return res;
}

Expand Down Expand Up @@ -3402,7 +3403,7 @@ TEE_Result syscall_asymm_operate(unsigned long state,
}

out:
free(params);
free_wipe(params);

if (res == TEE_SUCCESS || res == TEE_ERROR_SHORT_BUFFER) {
TEE_Result res2 = put_user_u64(dst_len, dlen);
Expand Down Expand Up @@ -3523,6 +3524,6 @@ TEE_Result syscall_asymm_verify(unsigned long state,
}

out:
free(params);
free_wipe(params);
return res;
}

0 comments on commit 70b6131

Please sign in to comment.