Re: [PATCH v4 3/4] fpga: dfl: add basic support DFHv1

From: matthew . gerlach
Date: Mon Oct 31 2022 - 16:15:13 EST




On Mon, 31 Oct 2022, Andy Shevchenko wrote:

On Mon, Oct 31, 2022 at 09:16:19AM +0800, Xu Yilun wrote:
On 2022-10-31 at 00:06:28 +0200, Andy Shevchenko wrote:
On Sat, Oct 29, 2022 at 09:08:44PM +0800, Xu Yilun wrote:
On 2022-10-20 at 14:26:09 -0700, matthew.gerlach@xxxxxxxxxxxxxxx wrote:

struct dfl_feature_info {
u16 fid;
u8 revision;
+ u8 dfh_version;
struct resource mmio_res;
void __iomem *ioaddr;
struct list_head node;
unsigned int irq_base;
unsigned int nr_irqs;
+ unsigned int param_size;
+ u64 params[];
};

...

+ finfo = kzalloc(sizeof(*finfo) + dfh_psize, GFP_KERNEL);


This probably may use something from overflow.h.

The u64 flexible array in the structure, but seems dfh_get_psize could
not garantee 64bit aligned size.

What's the mandatory alignment of param data? If 64bit aligned, bit 33-34
of PARAM_HDR should be reserved. If 32bit aligned, finfo:params should be
u32[].

Isn't it guaranteed by the C standard / architecture ABI?

I'm referring to the malloc size of the structure. It reserved dfh_psize
bytes for this u64 array, but there is no garantee dfh_psize should be a
multiple of 8. So there may be memory leak when accessing the last
array element?

Have you looked at macros in the overflow.h? Would the use of it solve your
concern?

By clarifying the definition of the next field in the parameter header as the number of 8-byte words, dfh_get_psize is guaranteed to be a multiple of 8.
This is fixed in the next revision of patches.

Matthew Gerlach


--
With Best Regards,
Andy Shevchenko