linux-kernel - Re: [PATCH 02/19] dma-buf-map: Add helper to initialize second map

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <f0dbdcc0-13b5-c484-0bf3-a1f8c3e48954@amd.com>
Date:   Thu, 27 Jan 2022 08:27:11 +0100
From:   Christian König <christian.koenig@....com>
To:     Lucas De Marchi <lucas.demarchi@...el.com>,
        intel-gfx@...ts.freedesktop.org
Cc:     dri-devel@...ts.freedesktop.org,
        Matt Roper <matthew.d.roper@...el.com>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        linux-media@...r.kernel.org, linaro-mm-sig@...ts.linaro.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/19] dma-buf-map: Add helper to initialize second map

Am 26.01.22 um 21:36 schrieb Lucas De Marchi:
> When dma_buf_map struct is passed around, it's useful to be able to
> initialize a second map that takes care of reading/writing to an offset
> of the original map.
>
> Add a helper that copies the struct and add the offset to the proper
> address.

Well what you propose here can lead to all kind of problems and is 
rather bad design as far as I can see.

The struct dma_buf_map is only to be filled in by the exporter and 
should not be modified in this way by the importer.

If you need to copy only a certain subset of the mapping use the 
functions you added in patch #1.

Regards,
Christian.

>
> Cc: Sumit Semwal <sumit.semwal@...aro.org>
> Cc: Christian König <christian.koenig@....com>
> Cc: linux-media@...r.kernel.org
> Cc: dri-devel@...ts.freedesktop.org
> Cc: linaro-mm-sig@...ts.linaro.org
> Cc: linux-kernel@...r.kernel.org
> Signed-off-by: Lucas De Marchi <lucas.demarchi@...el.com>
> ---
>   include/linux/dma-buf-map.h | 29 +++++++++++++++++++++++++++++
>   1 file changed, 29 insertions(+)
>
> diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
> index 65e927d9ce33..3514a859f628 100644
> --- a/include/linux/dma-buf-map.h
> +++ b/include/linux/dma-buf-map.h
> @@ -131,6 +131,35 @@ struct dma_buf_map {
>   		.is_iomem = false, \
>   	}
>   
> +/**
> + * DMA_BUF_MAP_INIT_OFFSET - Initializes struct dma_buf_map from another dma_buf_map
> + * @map_:	The dma-buf mapping structure to copy from
> + * @offset:	Offset to add to the other mapping
> + *
> + * Initializes a new dma_buf_struct based on another. This is the equivalent of doing:
> + *
> + * .. code-block: c
> + *
> + *	dma_buf_map map = other_map;
> + *	dma_buf_map_incr(&map, &offset);
> + *
> + * Example usage:
> + *
> + * .. code-block: c
> + *
> + *	void foo(struct device *dev, struct dma_buf_map *base_map)
> + *	{
> + *		...
> + *		struct dma_buf_map = DMA_BUF_MAP_INIT_OFFSET(base_map, FIELD_OFFSET);
> + *		...
> + *	}
> + */
> +#define DMA_BUF_MAP_INIT_OFFSET(map_, offset_)	(struct dma_buf_map)	\
> +	{								\
> +		.vaddr = (map_)->vaddr + (offset_),			\
> +		.is_iomem = (map_)->is_iomem,				\
> +	}
> +
>   /**
>    * dma_buf_map_set_vaddr - Sets a dma-buf mapping structure to an address in system memory
>    * @map:	The dma-buf mapping structure