[Linaro-mm-sig] Re: [PATCH v2 2/8] dma-buf: heaps: secure_heap: Add private heap ops

On Sat, Nov 11, 2023 at 3:16 AM Yong Wu yong.wu@mediatek.com wrote:

For the secure memory, there are two steps: a) Allocate buffers in kernel side; b) Secure that buffer. Different heaps may have different buffer allocation methods and different memory protection methods. Here abstract the memory allocation and securing operations.

Signed-off-by: Yong Wu yong.wu@mediatek.com

drivers/dma-buf/heaps/secure_heap.c | 58 ++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/heaps/secure_heap.c b/drivers/dma-buf/heaps/secure_heap.c index a634051a0a67..87ac23072e9e 100644 --- a/drivers/dma-buf/heaps/secure_heap.c +++ b/drivers/dma-buf/heaps/secure_heap.c @@ -24,15 +24,66 @@ struct secure_buffer { size_t size; };

+struct secure_heap;

+struct secure_heap_prv_data {

•   int     (*memory_alloc)(struct secure_heap *sec_heap, struct secure_buffer *sec_buf);
  

•   void    (*memory_free)(struct secure_heap *sec_heap, struct secure_buffer *sec_buf);
  

•   /* Protect/unprotect the memory */
  

•   int     (*secure_the_memory)(struct secure_heap *sec_heap, struct secure_buffer *sec_buf);
  

•   void    (*unsecure_the_memory)(struct secure_heap *sec_heap, struct secure_buffer *sec_buf);
  

+};

Move these into dma-heap-secure.h per the comments on the prior patch.

struct secure_heap { const char *name; const enum secure_memory_type mem_type;

•   const struct secure_heap_prv_data *data;
  

};

+static int secure_heap_secure_memory_allocate(struct secure_heap *sec_heap,

•                                         struct secure_buffer *sec_buf)
  

+{

•   const struct secure_heap_prv_data *data = sec_heap->data;
  

•   int ret;
  

•   if (data->memory_alloc) {
  

•           ret = data->memory_alloc(sec_heap, sec_buf);
  

•           if (ret)
  

•                   return ret;
  

•   }
  

You should probably always require that memory_alloc is defined (secure_the_memory can be optional, as that may be part of the allocation).

•   if (data->secure_the_memory) {
  

•           ret = data->secure_the_memory(sec_heap, sec_buf);
  

•           if (ret)
  

•                   goto sec_memory_free;
  

•   }
  

•   return 0;
  

+sec_memory_free:

•   if (data->memory_free)
  

•           data->memory_free(sec_heap, sec_buf);
  

You should probably always require that memory_free is defined.

•   return ret;
  

+}

+static void secure_heap_secure_memory_free(struct secure_heap *sec_heap,

•                                      struct secure_buffer *sec_buf)
  

+{

•   const struct secure_heap_prv_data *data = sec_heap->data;
  

•   if (data->unsecure_the_memory)
  

•           data->unsecure_the_memory(sec_heap, sec_buf);
  

•   if (data->memory_free)
  

•           data->memory_free(sec_heap, sec_buf);
  

You should probably always require that memory_free is defined.

+}

static struct dma_buf * secure_heap_allocate(struct dma_heap *heap, unsigned long size, unsigned long fd_flags, unsigned long heap_flags) {

•   struct secure_heap *sec_heap = dma_heap_get_drvdata(heap);
    struct secure_buffer *sec_buf;
    DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
    struct dma_buf *dmabuf;
  

@@ -45,6 +96,9 @@ secure_heap_allocate(struct dma_heap *heap, unsigned long size, sec_buf->size = ALIGN(size, PAGE_SIZE); sec_buf->heap = heap;

•   ret = secure_heap_secure_memory_allocate(sec_heap, sec_buf);
  

•   if (ret)
  

•           goto err_free_buf;
    exp_info.exp_name = dma_heap_get_name(heap);
    exp_info.size = sec_buf->size;
    exp_info.flags = fd_flags;
  

@@ -53,11 +107,13 @@ secure_heap_allocate(struct dma_heap *heap, unsigned long size, dmabuf = dma_buf_export(&exp_info); if (IS_ERR(dmabuf)) { ret = PTR_ERR(dmabuf);

•           goto err_free_buf;
  

•           goto err_free_sec_mem;
    }
  
    return dmabuf;
  
  

+err_free_sec_mem:

•   secure_heap_secure_memory_free(sec_heap, sec_buf);
  

err_free_buf: kfree(sec_buf); return ERR_PTR(ret); -- 2.25.1