| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 28 Jun 2024 14:38:18 +0200
From: Thierry Reding <thierry.reding@...il.com>
To: Yong Wu <yong.wu@...iatek.com>
Cc: Rob Herring <robh+dt@...nel.org>,
Matthias Brugger <matthias.bgg@...il.com>, christian.koenig@....com, Sumit Semwal <sumit.semwal@...aro.org>,
Andrew Morton <akpm@...ux-foundation.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Benjamin Gaignard <benjamin.gaignard@...labora.com>,
Brian Starkey <Brian.Starkey@....com>, John Stultz <jstultz@...gle.com>, tjmercier@...gle.com,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org, linaro-mm-sig@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
Robin Murphy <robin.murphy@....com>, Vijayanand Jitta <quic_vjitta@...cinc.com>,
Joakim Bech <joakim.bech@...aro.org>, Jeffrey Kardatzke <jkardatzke@...gle.com>,
Pavel Machek <pavel@....cz>, Simon Ser <contact@...rsion.fr>,
Pekka Paalanen <ppaalanen@...il.com>, willy@...radead.org, Logan Gunthorpe <logang@...tatee.com>,
Daniel Vetter <daniel@...ll.ch>, jianjiao.zeng@...iatek.com, kuohong.wang@...iatek.com,
youlin.pei@...iatek.com
Subject: Re: [PATCH v5 7/9] dma-buf: heaps: restricted_heap: Add MediaTek
restricted heap and heap_init
On Wed, May 15, 2024 at 07:23:06PM GMT, Yong Wu wrote:
> Add a MediaTek restricted heap which uses TEE service call to restrict
> buffer. Currently this restricted heap is NULL, Prepare for the later
> patch. Mainly there are two changes:
> a) Add a heap_init ops since TEE probe late than restricted heap, thus
> initialize the heap when we require the buffer the first time.
> b) Add a priv_data for each heap, like the special data used by MTK
> (such as "TEE session") can be placed in priv_data.
>
> Currently our heap depends on CMA which could only be bool, thus
> depend on "TEE=y".
>
> Signed-off-by: Yong Wu <yong.wu@...iatek.com>
> ---
> drivers/dma-buf/heaps/Kconfig | 7 ++
> drivers/dma-buf/heaps/Makefile | 1 +
> drivers/dma-buf/heaps/restricted_heap.c | 11 ++
> drivers/dma-buf/heaps/restricted_heap.h | 2 +
> drivers/dma-buf/heaps/restricted_heap_mtk.c | 115 ++++++++++++++++++++
> 5 files changed, 136 insertions(+)
> create mode 100644 drivers/dma-buf/heaps/restricted_heap_mtk.c
>
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index e54506f480ea..84f748fb2856 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -21,3 +21,10 @@ config DMABUF_HEAPS_RESTRICTED
> heap is to manage buffers that are inaccessible to the kernel and user space.
> There may be several ways to restrict it, for example it may be encrypted or
> protected by a TEE or hypervisor. If in doubt, say N.
> +
> +config DMABUF_HEAPS_RESTRICTED_MTK
> + bool "MediaTek DMA-BUF Restricted Heap"
> + depends on DMABUF_HEAPS_RESTRICTED && TEE=y
> + help
> + Enable restricted dma-buf heaps for MediaTek platform. This heap is backed by
> + TEE client interfaces. If in doubt, say N.
> diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> index a2437c1817e2..0028aa9d875f 100644
> --- a/drivers/dma-buf/heaps/Makefile
> +++ b/drivers/dma-buf/heaps/Makefile
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o
> obj-$(CONFIG_DMABUF_HEAPS_RESTRICTED) += restricted_heap.o
> +obj-$(CONFIG_DMABUF_HEAPS_RESTRICTED_MTK) += restricted_heap_mtk.o
> obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) += system_heap.o
> diff --git a/drivers/dma-buf/heaps/restricted_heap.c b/drivers/dma-buf/heaps/restricted_heap.c
> index 4e45d46a6467..8bc8a5e3f969 100644
> --- a/drivers/dma-buf/heaps/restricted_heap.c
> +++ b/drivers/dma-buf/heaps/restricted_heap.c
> @@ -151,11 +151,22 @@ restricted_heap_allocate(struct dma_heap *heap, unsigned long size,
> unsigned long fd_flags, unsigned long heap_flags)
> {
> struct restricted_heap *rheap = dma_heap_get_drvdata(heap);
> + const struct restricted_heap_ops *ops = rheap->ops;
> struct restricted_buffer *restricted_buf;
> DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> struct dma_buf *dmabuf;
> int ret;
>
> + /*
> + * In some implements, TEE is required to protect buffer. However TEE probe
> + * may be late, Thus heap_init is performed when the first buffer is requested.
> + */
> + if (ops->heap_init) {
> + ret = ops->heap_init(rheap);
> + if (ret)
> + return ERR_PTR(ret);
> + }
I wonder if we should make this parameterized rather than the default.
Perhaps we can add a "init_on_demand" (or whatever other name) flag to
struct restricted_heap_ops and then call this from heap initialization
if possible and defer initialization depending on the restricted heap
provider?
> +
> restricted_buf = kzalloc(sizeof(*restricted_buf), GFP_KERNEL);
> if (!restricted_buf)
> return ERR_PTR(-ENOMEM);
> diff --git a/drivers/dma-buf/heaps/restricted_heap.h b/drivers/dma-buf/heaps/restricted_heap.h
> index 6d9599a4a34e..2a33a1c7a48b 100644
> --- a/drivers/dma-buf/heaps/restricted_heap.h
> +++ b/drivers/dma-buf/heaps/restricted_heap.h
> @@ -19,6 +19,8 @@ struct restricted_heap {
> const char *name;
>
> const struct restricted_heap_ops *ops;
> +
> + void *priv_data;
Honestly, I would just get rid of any of this extra padding/indentation
in these structures. There's really no benefit to this, except maybe if
you *really* like things to be aligned, in which case the above is now
probably worse than if you didn't try to align in the first place.
Thierry
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists