Hi, > Date: 2026-10-04 17:11:18-0700 > From: Rosen Penev <[email protected]> > > 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 <[email protected]> > --- > 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>
signature.asc
Description: PGP signature
