On Tue, 20 Dec 2022, Andy Shevchenko wrote:
On Tue, Dec 20, 2022 at 08:36:51AM -0800, matthew.gerlach@xxxxxxxxxxxxxxx wrote:
From: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
Version 1 of the Device Feature Header (DFH) definition adds
functionality to the DFL bus.
A DFHv1 header may have one or more parameter blocks that
further describes the HW to SW. Add support to the DFL bus
to parse the MSI-X parameter.
The location of a feature's register set is explicitly
described in DFHv1 and can be relative to the base of the DFHv1
or an absolute address. Parse the location and pass the information
to DFL driver.
...
+/**
+ * dfh_find_param() - find data for the given parameter id
+ * @dfl_dev: dfl device
+ * @param: id of dfl parameter
+ *
+ * Return: pointer to parameter header on success, NULL otherwise.
header is a bit confusing here, does it mean we give and ID and we got
something more than just a data as summary above suggests?
Yes, the summary is not correct. It should say "find the parameter block
for the given parameter id".
In such case summary and this text should clarify what exactly we get
and layout of the data. Since this is a pointer, who is responsible of
checking out-of-boundary accesses? For instance, if the parameters are
variadic by length the length should be returned as well. Otherwise it
should be specified as a constant somewhere, right?
The parameter header has the next/size field; so the caller of
dfh_find_param should perform boundary checking as part of interpreting
the parameter data. I think a function to perform this checking
and data interpretation would help here.
+ */
+u64 *dfh_find_param(struct dfl_device *dfl_dev, int param_id)
+{
+ return find_param(dfl_dev->params, dfl_dev->param_size, param_id);
+}
+EXPORT_SYMBOL_GPL(dfh_find_param);
...
+ finfo = kzalloc(sizeof(*finfo) + dfh_psize, GFP_KERNEL);
It sounds like a candidate for struct_size() from overflow.h.
I.o.w. check that header and come up with the best what can
suit your case.
finfo = kzalloc(struct_size(finfo, params, dfh_psize/sizeof(u64)),
GFP_KERNEL);
Does seem better.
Thanks for the suggestion,
Matthew Gerlach
if (!finfo)
return -ENOMEM;
--
With Best Regards,
Andy Shevchenko
