* *allocflex() (was: [PATCH] drm/exynos: fimc: allocate formats as a flexible array member)
2026-10-05 0:11 [PATCH] drm/exynos: fimc: allocate formats as a flexible array member Rosen Penev
@ 2026-10-05 8:07 ` Alejandro Colomar
0 siblings, 0 replies; 2+ messages in thread
From: Alejandro Colomar @ 2026-10-05 8:07 UTC (permalink / raw)
To: Rosen Penev, Kees Cook
Cc: dri-devel, Inki Dae, Seung-Woo Kim, Kyungmin Park, David Airlie,
Simona Vetter, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar,
Kees Cook, Gustavo A. R. Silva, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list,
open list:KERNEL HARDENING (not covered by other
areas):Keyword:b__counted_by(_le|_be|_ptr)?b
[-- 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 --]
^ permalink raw reply [flat|nested] 2+ messages in thread