linux-kernel - Re: [PATCH 0/3] dma_mapping: Add auto cleanup support

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: <f3fba346-6fdd-4b0e-9414-087a12772a6a@samsung.com>
Date: Tue, 14 Oct 2025 07:54:46 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Frank Li <Frank.Li@....com>, Robin Murphy <robin.murphy@....com>, Dong
	Aisheng <aisheng.dong@....com>, Andi Shyti <andi.shyti@...nel.org>, Shawn
	Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam
	<festevam@...il.com>, Peter Zijlstra <peterz@...radead.org>
Cc: linux-kernel@...r.kernel.org, iommu@...ts.linux.dev,
	linux-i2c@...r.kernel.org, imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 0/3] dma_mapping: Add auto cleanup support

Hi

On 10.10.2025 20:50, Frank Li wrote:
> There are many below pattern
>
> 	fun()
> 	{
> 		...
> 		dma_map_single();
> 		if (dma_mapping_error)
> 			goto err1;
>
> 		dmaengine_prep_slave_single()
> 		if (...)
> 			goto err2
>
> 		dmaengine_submit()
> 		if (...)
> 			goto err3
>
> 		wait_for_completion_timeout()
> 		if (...)
> 			goto err4
>
> 	err4:
> 	err3:
> 	err2:
> 		dma_umap_single();
> 	err1:
> 	}
>
> Use cleanup can simple error handle like guard(), such as guard(mutex).
> or __free(kfree) = kmalloc.
>
> But dma_umap_single() need more argurements. So situation below complex.
>
> It need pack argurments list into structure.
>
> 	#define __DEFINE_GUARD_CLASS(_name, _return_type, _list_class_fields)   \
> 	typedef struct {                                                \
> 	       	 _return_type ret;                                       \
> 	        bool    okay;                                           \
> 	        struct  {                                               \
> 	                __REMOVE(_list_class_fields);                   \
> 	        } args;                                                 \
> 	} class_##_name##_t;
>
> So save all arugments to it.
>
> __DEFINE_GUARD_CLASS(dma_map_single, dma_addr_t, (struct device *dev; void *ptr; size_t size; enum dma_data_direction dir)
> will expand to
>
> 	struct {
> 		dma_addr_t ret;
> 		bool	okay;
> 		struct {
> 			struct device *dev;
> 			void *ptr;
> 			size_t size;
> 			enum dma_data_direction dir;
> 		}
> 	}
>
> So cleanup function can use saved argurement.
>
> The above fun will be
>
> 	fun()
> 	{
> 		CLASS(dma_map_single, dma)(dev, ...);
>
> 		...
> 		if (...)
> 			return err;
> 	}
>
> if funtion return, which need keep map,
>
> 	submit()
> 	{
> 		dma_map_single();
> 		...
> 		dmaengine_submit();
> 		if (...)
> 			goto err1
> 		return;
>
> 	goto err1:
> 		dma_umap_single();
> 	}
>
> Macro retain_and_empty() will clean varible to avoid unmap.
>
>          ({                                  \
>                  __auto_type __ptr = &(t); typeof(t) empty= {};   \
>                  __auto_type __val = *__ptr; \
>                  __ptr->okay = 0;        \
>                  __val.ret;              \
>          })
>
> So
>
> 	submit()
> 	{
>         		CLASS(dma_map_single, dma)(dev,...;
>
> 	        ...
> 	        dmaengine_submit();
> 		if (...)
> 			return err;
>
> 		//before return;
>
> 		retain_and_empty(dma)
> 	}
>
> This series just show how to hanndle many agurement at resource alloc/free
> functions. Only show dma_map_single. If the over all method is acceptable.
> I will more define for dma mapping functions.
>
> Signed-off-by: Frank Li <Frank.Li@....com>

This looks fine from the DMA-mapping API perspective. I think we should 
also get some feedback from Peter, who authored most of the __cleanup() 
based infrastructure, so I've added him to the recipients list.


> ---
> Frank Li (3):
>        cleanup: Add DEFINE_GUARD_ARGS_CLASS macro for resource alloc/free functions with multiple arguments
>        dma-mapping: Add auto cleanup support dma_map_single()
>        i2c: lpi2c: Use auto cleanup for dma_map_single()
>
>   drivers/i2c/busses/i2c-imx-lpi2c.c | 13 ++++---
>   include/linux/cleanup.h            | 73 ++++++++++++++++++++++++++++++++++++++
>   include/linux/dma-mapping.h        |  8 +++++
>   3 files changed, 87 insertions(+), 7 deletions(-)
> ---
> base-commit: 7c3ba4249a3604477ea9c077e10089ba7ddcaa03
> change-id: 20251008-dmamap_cleanup-d0a7f0525a3d
>
> Best regards,
> --
> Frank Li <Frank.Li@....com>
>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland