linux-kernel - Re: [PATCH v3 3/3] dma-buf: heaps: Give default CMA heap a fixed name

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250527-gainful-jaguar-of-honor-a72e15@houat>
Date: Tue, 27 May 2025 15:05:08 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Jared Kangas <jkangas@...hat.com>
Cc: sumit.semwal@...aro.org, benjamin.gaignard@...labora.com, 
	Brian.Starkey@....com, jstultz@...gle.com, tjmercier@...gle.com, 
	christian.koenig@....com, linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	linaro-mm-sig@...ts.linaro.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] dma-buf: heaps: Give default CMA heap a fixed name

Hi,

On Thu, May 22, 2025 at 12:14:18PM -0700, Jared Kangas wrote:
> The CMA heap's name in devtmpfs can vary depending on how the heap is
> defined. Its name defaults to "reserved", but if a CMA area is defined
> in the devicetree, the heap takes on the devicetree node's name, such as
> "default-pool" or "linux,cma". To simplify naming, unconditionally name
> it "default_cma_region", but keep a legacy node in place backed by the
> same underlying allocator for backwards compatibility.
> 
> Signed-off-by: Jared Kangas <jkangas@...hat.com>
> ---
>  Documentation/userspace-api/dma-buf-heaps.rst |  7 +++++--
>  drivers/dma-buf/heaps/Kconfig                 | 10 ++++++++++
>  drivers/dma-buf/heaps/cma_heap.c              | 20 ++++++++++++++++++-
>  3 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
> index 23bd0bd7b0654..1dfe5e7acd5a3 100644
> --- a/Documentation/userspace-api/dma-buf-heaps.rst
> +++ b/Documentation/userspace-api/dma-buf-heaps.rst
> @@ -21,5 +21,8 @@ following heaps:
>     usually created either through the kernel commandline through the
>     ``cma`` parameter, a memory region Device-Tree node with the
>     ``linux,cma-default`` property set, or through the ``CMA_SIZE_MBYTES`` or
> -   ``CMA_SIZE_PERCENTAGE`` Kconfig options. Depending on the platform, it
> -   might be called ``reserved``, ``linux,cma``, or ``default-pool``.
> +   ``CMA_SIZE_PERCENTAGE`` Kconfig options. The heap's name in devtmpfs is
> +   ``default_cma_region``. For backwards compatibility, when the
> +   ``DMABUF_HEAPS_CMA_LEGACY`` Kconfig option is set, a duplicate node is
> +   created following legacy naming conventions; the legacy name might be
> +   ``reserved``, ``linux,cma``, or ``default-pool``.
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index a5eef06c42264..bb369b38b001a 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -12,3 +12,13 @@ config DMABUF_HEAPS_CMA
>  	  Choose this option to enable dma-buf CMA heap. This heap is backed
>  	  by the Contiguous Memory Allocator (CMA). If your system has these
>  	  regions, you should say Y here.
> +
> +config DMABUF_HEAPS_CMA_LEGACY
> +	bool "Legacy DMA-BUF CMA Heap"
> +	default y
> +	depends on DMABUF_HEAPS_CMA
> +	help
> +	  Add a duplicate CMA-backed dma-buf heap with legacy naming derived
> +	  from the CMA area's devicetree node, or "reserved" if the area is not
> +	  defined in the devicetree. This uses the same underlying allocator as
> +	  CONFIG_DMABUF_HEAPS_CMA.
> diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
> index e998d8ccd1dc6..dfeccafc6ae3c 100644
> --- a/drivers/dma-buf/heaps/cma_heap.c
> +++ b/drivers/dma-buf/heaps/cma_heap.c
> @@ -9,6 +9,9 @@
>   * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
>   *	Andrew F. Davis <afd@...com>
>   */
> +
> +#define pr_fmt(fmt) "cma_heap: " fmt
> +
>  #include <linux/cma.h>
>  #include <linux/dma-buf.h>
>  #include <linux/dma-heap.h>
> @@ -22,6 +25,7 @@
>  #include <linux/slab.h>
>  #include <linux/vmalloc.h>
>  
> +#define DEFAULT_CMA_NAME "default_cma_region"
>  
>  struct cma_heap {
>  	struct dma_heap *heap;
> @@ -394,15 +398,29 @@ static int __init __add_cma_heap(struct cma *cma, const char *name)
>  static int __init add_default_cma_heap(void)
>  {
>  	struct cma *default_cma = dev_get_cma_area(NULL);
> +	const char *legacy_cma_name;
>  	int ret;
>  
>  	if (!default_cma)
>  		return 0;
>  
> -	ret = __add_cma_heap(default_cma, cma_get_name(default_cma));
> +	ret = __add_cma_heap(default_cma, DEFAULT_CMA_NAME);
>  	if (ret)
>  		return ret;
>  
> +	if (IS_ENABLED(CONFIG_DMABUF_HEAPS_CMA_LEGACY)) {
> +		legacy_cma_name = cma_get_name(default_cma);
> +		if (!strcmp(legacy_cma_name, DEFAULT_CMA_NAME)) {
> +			pr_warn("legacy name and default name are the same, skipping legacy heap\n");
> +			return 0;
> +		}
> +
> +		ret = __add_cma_heap(default_cma, legacy_cma_name);
> +		if (ret)
> +			pr_warn("failed to add legacy heap: %pe\n",
> +				ERR_PTR(-ret));

Are you sure about the -ret? ret should already be a negative number if it failed?

With that fixed,

Reviewed-by: Maxime Ripard <mripard@...nel.org>

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)