From dcdd5691d94501dbb1552fbf3fbf6c929101e153 Mon Sep 17 00:00:00 2001 From: "Isaac J. Manjarres" Date: Tue, 7 Apr 2020 11:20:39 -0700 Subject: [PATCH 1/3] ion: Improve ION allocation paths Clean up some of the ION heap interfaces so that they are more inline/uniform with respect to each other. Change-Id: I4edabc2c8ccb533898540ceda1fd6aacc2e2e56a Signed-off-by: Isaac J. Manjarres --- .../staging/android/ion/heaps/ion_cma_heap.c | 18 +++++++++++++++--- .../android/ion/heaps/ion_secure_util.c | 5 +++++ .../android/ion/heaps/ion_secure_util.h | 4 +++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/drivers/staging/android/ion/heaps/ion_cma_heap.c b/drivers/staging/android/ion/heaps/ion_cma_heap.c index b91320c4212c..8a8c7dca9f4d 100644 --- a/drivers/staging/android/ion/heaps/ion_cma_heap.c +++ b/drivers/staging/android/ion/heaps/ion_cma_heap.c @@ -5,7 +5,7 @@ * Copyright (C) Linaro 2012 * Author: for ST-Ericsson. * - * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved. */ #include @@ -29,6 +29,11 @@ struct ion_cma_heap { container_of(to_msm_ion_heap(x), struct ion_cma_heap, heap) /* ION CMA heap operations functions */ +static bool ion_heap_is_cma_heap_type(enum ion_heap_type type) +{ + return type == ION_HEAP_TYPE_DMA; +} + static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, unsigned long len, unsigned long flags) @@ -42,6 +47,13 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, int ret; struct device *dev = cma_heap->heap.dev; + if (ion_heap_is_cma_heap_type(buffer->heap->type) && + is_secure_allocation(buffer->flags)) { + pr_err("%s: CMA heap doesn't support secure allocations\n", + __func__); + return -EINVAL; + } + if (align > CONFIG_CMA_ALIGNMENT) align = CONFIG_CMA_ALIGNMENT; @@ -49,7 +61,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, if (!pages) return -ENOMEM; - if (!(flags & ION_FLAG_SECURE)) { + if (hlos_accessible_buffer(buffer)) { if (PageHighMem(pages)) { unsigned long nr_clear_pages = nr_pages; struct page *page = pages; @@ -68,7 +80,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, } if (MAKE_ION_ALLOC_DMA_READY || - (flags & ION_FLAG_SECURE) || + (!hlos_accessible_buffer(buffer)) || (!ion_buffer_cached(buffer))) ion_pages_sync_for_device(dev, pages, size, DMA_BIDIRECTIONAL); diff --git a/drivers/staging/android/ion/heaps/ion_secure_util.c b/drivers/staging/android/ion/heaps/ion_secure_util.c index 7f871cd768ad..88cc3a6e457f 100644 --- a/drivers/staging/android/ion/heaps/ion_secure_util.c +++ b/drivers/staging/android/ion/heaps/ion_secure_util.c @@ -31,6 +31,11 @@ bool is_secure_vmid_valid(int vmid) (!ret && vmid == trusted_vm_vmid)); } +bool is_secure_allocation(unsigned long flags) +{ + return !!(flags & (ION_FLAGS_CP_MASK | ION_FLAG_SECURE)); +} + int get_secure_vmid(unsigned long flags) { int ret; diff --git a/drivers/staging/android/ion/heaps/ion_secure_util.h b/drivers/staging/android/ion/heaps/ion_secure_util.h index e0b94debd146..523ca279e0a3 100644 --- a/drivers/staging/android/ion/heaps/ion_secure_util.h +++ b/drivers/staging/android/ion/heaps/ion_secure_util.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. */ #include "msm_ion_priv.h" @@ -22,4 +22,6 @@ int ion_hyp_assign_from_flags(u64 base, u64 size, unsigned long flags); bool hlos_accessible_buffer(struct ion_buffer *buffer); +bool is_secure_allocation(unsigned long flags); + #endif /* _ION_SECURE_UTIL_H */ From 90e828489772f705fcd54b562e3dbf9ba312b4a6 Mon Sep 17 00:00:00 2001 From: "Isaac J. Manjarres" Date: Tue, 14 Apr 2020 12:03:41 -0700 Subject: [PATCH 2/3] ion: Forbid multi-VMID allocation requests for the secure system heap The secure system heap does not support multi-VMID allocations, so do not allow them. Also, fortify the secure allocation check for the system heap allocation path. Change-Id: I51d53274c2374632db778461ba7505de4d723439 Signed-off-by: Isaac J. Manjarres --- drivers/staging/android/ion/heaps/ion_system_heap.c | 2 +- drivers/staging/android/ion/heaps/ion_system_secure_heap.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/ion/heaps/ion_system_heap.c b/drivers/staging/android/ion/heaps/ion_system_heap.c index 4885702f5d16..e0cc4bab86b8 100644 --- a/drivers/staging/android/ion/heaps/ion_system_heap.c +++ b/drivers/staging/android/ion/heaps/ion_system_heap.c @@ -288,7 +288,7 @@ static int ion_system_heap_allocate(struct ion_heap *heap, return -ENOMEM; if (ion_heap_is_system_heap_type(buffer->heap->type) && - is_secure_vmid_valid(vmid)) { + is_secure_allocation(buffer->flags)) { pr_info("%s: System heap doesn't support secure allocations\n", __func__); return -EINVAL; diff --git a/drivers/staging/android/ion/heaps/ion_system_secure_heap.c b/drivers/staging/android/ion/heaps/ion_system_secure_heap.c index 188b28f6c5d5..7b252d7ad169 100644 --- a/drivers/staging/android/ion/heaps/ion_system_secure_heap.c +++ b/drivers/staging/android/ion/heaps/ion_system_secure_heap.c @@ -79,9 +79,10 @@ static int ion_system_secure_heap_allocate(struct ion_heap *heap, struct ion_system_secure_heap *secure_heap = to_system_secure_heap(heap); enum ion_heap_type type = secure_heap->heap.ion_heap.type; + unsigned long cp_flags = buffer->flags & ION_FLAGS_CP_MASK; if (!ion_heap_is_system_secure_heap_type(type) || - !is_cp_flag_present(flags)) { + !is_cp_flag_present(flags) || (hweight_long(cp_flags) != 1)) { pr_info("%s: Incorrect heap type or incorrect flags\n", __func__); return -EINVAL; From 3d85b42ad19d49ae40014746c3ea48a73c646eab Mon Sep 17 00:00:00 2001 From: "Isaac J. Manjarres" Date: Thu, 16 Apr 2020 10:08:05 -0700 Subject: [PATCH 3/3] ion: Ensure secure HLOS accessible buffers are zeroed when allocated Memory that is either pooled or permanently assigned to other VMIDs is not always cleared when it is released back into the pools. This is problematic when the memory is assigned such that HLOS can still access the memory, as a userspace process can reallocate the same buffer, and read data from when the buffer was previously allocated. Thus, clear secure ION buffers before freeing them, if they are HLOS accessible. Change-Id: Iaae14fc32b10e0c6afca007a5ee78aacd5e013e6 Signed-off-by: Isaac J. Manjarres --- drivers/staging/android/ion/heaps/ion_carveout_heap.c | 2 ++ drivers/staging/android/ion/heaps/ion_system_heap.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/android/ion/heaps/ion_carveout_heap.c b/drivers/staging/android/ion/heaps/ion_carveout_heap.c index 7aaef6a9d6c8..53347da25295 100644 --- a/drivers/staging/android/ion/heaps/ion_carveout_heap.c +++ b/drivers/staging/android/ion/heaps/ion_carveout_heap.c @@ -379,6 +379,8 @@ static void ion_sc_heap_free(struct ion_buffer *buffer) return; } + if (hlos_accessible_buffer(buffer)) + ion_buffer_zero(buffer); ion_carveout_free(child, paddr, buffer->size); sg_free_table(table); kfree(table); diff --git a/drivers/staging/android/ion/heaps/ion_system_heap.c b/drivers/staging/android/ion/heaps/ion_system_heap.c index e0cc4bab86b8..6c9bfc8ff4b5 100644 --- a/drivers/staging/android/ion/heaps/ion_system_heap.c +++ b/drivers/staging/android/ion/heaps/ion_system_heap.c @@ -426,7 +426,7 @@ void ion_system_heap_free(struct ion_buffer *buffer) if (!(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE) && !(buffer->flags & ION_FLAG_POOL_FORCE_ALLOC)) { - if (vmid < 0) + if (hlos_accessible_buffer(buffer)) ion_buffer_zero(buffer); } else if (vmid > 0) { if (ion_hyp_unassign_sg(table, &vmid, 1, true))