*allocflex() (was: [PATCH] drm/exynos: fimc: allocate formats as a flexible array member)

[Alejandro Colomar <alx+linux-hardening@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 5544 bytes --]

Hi,

> Date: 2026-10-04 17:11:18-0700
> From: Rosen Penev <rosenp@gmail.com>
>
> The formats array is allocated separately with devm_kcalloc() and only
> referenced through the fimc_context. Make it a flexible array member at
> the end of the context and allocate both in one go with struct_size().
> 
> Annotate it with __counted_by(num_formats) so the array accesses can be
> bounds checked with FORTIFY_SOURCE and UBSAN_BOUNDS.
> 
> Assisted-by: LLM
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimc.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> index 09e33a26caaf..079d24d0671c 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -99,7 +99,6 @@ struct fimc_context {
>  	void		*dma_priv;
>  	struct device	*dev;
>  	struct exynos_drm_ipp_task	*task;
> -	struct exynos_drm_ipp_formats	*formats;
>  	unsigned int			num_formats;
>  
>  	void __iomem	*regs;
> @@ -108,6 +107,7 @@ struct fimc_context {
>  	struct fimc_scaler	sc;
>  	int	id;
>  	int	irq;
> +	struct exynos_drm_ipp_formats	formats[] __counted_by(num_formats);
>  };
>  
>  static u32 fimc_read(struct fimc_context *ctx, u32 reg)
> @@ -1273,20 +1273,16 @@ static int fimc_probe(struct platform_device *pdev)
>  	if (exynos_drm_check_fimc_device(dev) != 0)
>  		return -ENODEV;
>  
> -	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	num_formats = ARRAY_SIZE(fimc_formats) + ARRAY_SIZE(fimc_tiled_formats);
> +	ctx = devm_kzalloc(dev, struct_size(ctx, formats, num_formats), GFP_KERNEL);

Unrelated to this patch, but this has triggered my attention.

I have been recently researching into allocation of FAMs, trying to make
them more type safe, and found it safer to have a specialized API for
such allocations, whose pseudo-prototype would look like this:

	T *mallocflex(typename S, size_t n, flex);

A devm_kzallocflex() could then have an extra 'dev' parameter at the
front, and the usual 'gpf' one at the end.  It could be used here as:

	n = ARRAY_SIZE(fimc_formats) + ARRAY_SIZE(fimc_tiled_formats);
	ctx = devm_kzallocflex(dev, struct fimc_context, n, formats, GFP_KERNEL);

Kees, do you think we could add something like this?

Here's a small program showing how it could be implemented:

	$ cat fam.c 
	#include <errno.h>
	#include <stddef.h>
	#include <stdint.h>
	#include <stdio.h>
	#include <stdlib.h>
	#include <sys/param.h>

	#define typeof_typename(T)      typeof(*(typeof(T) *){_Generic(0, T: NULL, default: NULL)})
	#define memberof(T, member)     ((T){}.member)
	#define sizeof_flex(T, n, fam)  MAX(sizeof(T), offsetof(T, fam[n]))
	#define ptr_cast(T, p)          rvalue((typeof_typename(T) *){(p)})
	#define rvalue(lv)              ((void)0, (lv))

	#define reallocflex_T(p, S, n, flex)   reallocflex_T_(p, typeof_typename(S), n, flex)
	#define reallocflex_T_(p, S, n, flex)                         \
	(                                                             \
		_Generic(p, S *: (void)0),                            \
		ptr_cast(S, reallocflex_T__(p, S, n, flex))           \
	)
	#define reallocflex_T__(p, S, n, f)                           \
	(                                                             \
		reallocflex(p, sizeof(S), offsetof(S, f), n, sizeof(memberof(S, f)[0]))\
	)
	#define mallocflex_T(S, n, flex)                              \
	(                                                             \
		ptr_cast(S, reallocflex_T__(NULL, S, n, flex))        \
	)


	void *
	reallocflex(void *p, size_t headsize, size_t off, size_t n, size_t eltsize)
	{
		if (n != 0 && eltsize > SIZE_MAX / n) {
			errno = ENOMEM;
			return NULL;
		}
		if (off > SIZE_MAX - n*eltsize) {
			errno = ENOMEM;
			return NULL;
		}
		if (off > headsize) {
			errno = ENOMEM;
			return NULL;
		}

	#ifndef NDEBUG
		printf("%zu (%zu, %zu, %zu, %zu)\n", MAX(headsize, off + n * eltsize), headsize, off, n, eltsize);
	#endif

		return realloc(p, MAX(headsize, off + n * eltsize));
	}

	struct s {
		long a;
		char b,c,d;
		int f[];
	};

	int
	main(void)
	{
		struct s  *p;

		printf("s: %zu\n", sizeof(struct s));
		printf("o: %zu\n", offsetof(struct s, f));

		p = mallocflex_T(struct s, 2, f);
		p = reallocflex_T(p, struct s, 0, f);
		p = reallocflex_T(p, struct s, 1, f);

		bzero(p, sizeof_flex(struct s, 1, f));

		free(p);
	}


Have a lovely day!
Alex


>  	if (!ctx)
>  		return -ENOMEM;
>  
> +	ctx->num_formats = num_formats;
> +	formats = ctx->formats;
>  	ctx->dev = dev;
>  	ctx->id = of_alias_get_id(dev->of_node, "fimc");
>  
> -	/* construct formats/limits array */
> -	num_formats = ARRAY_SIZE(fimc_formats) + ARRAY_SIZE(fimc_tiled_formats);
> -	formats = devm_kcalloc(dev, num_formats, sizeof(*formats),
> -			       GFP_KERNEL);
> -	if (!formats)
> -		return -ENOMEM;
> -
>  	/* linear formats */
>  	if (ctx->id < 3) {
>  		limits = fimc_4210_limits_v1;
> @@ -1320,9 +1316,6 @@ static int fimc_probe(struct platform_device *pdev)
>  		formats[j].num_limits = num_limits;
>  	}
>  
> -	ctx->formats = formats;
> -	ctx->num_formats = num_formats;
> -
>  	/* resource memory */
>  	ctx->regs = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(ctx->regs))
> -- 
> 2.56.0
> 
> 

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]