Messages in this thread | | | Date | Sat, 29 Jul 2023 12:46:18 +0100 | From | Jonathan Cameron <> | Subject | Re: [PATCH v3 2/4] iio: core: Add opaque_struct_size() helper and use it |
| |
On Mon, 24 Jul 2023 14:02:02 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> Introduce opaque_struct_size() helper, which may be moved > to overflow.h in the future, and use it in the IIO core. > > Potential users could be (among possible others): > > __spi_alloc_controller() in drivers/spi/spi.c > alloc_netdev_mqs in net/core/dev.c > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Cc: Kees Cook <keescook@chromium.org> > Reviewed-by: Nuno Sa <nuno.sa@analog.com> > --- > drivers/iio/industrialio-core.c | 28 ++++++++++++++++++++-------- > 1 file changed, 20 insertions(+), 8 deletions(-) > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index b153adc5bc84..118ca6b59504 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -1607,6 +1607,24 @@ const struct device_type iio_device_type = { > .release = iio_dev_release, > }; > > +/** > + * opaque_struct_size() - Calculate size of opaque structure with trailing aligned data. > + * @p: Pointer to the opaque structure. > + * @a: Alignment in bytes before trailing data. > + * @s: Data size in bytes (preferred not to be 0). > + * > + * Calculates size of memory needed for structure of @p followed by > + * the aligned data of size @s. > + * > + * Note, when @s is 0, the alignment @a is added to the sizeof(*(@p)) > + * and the result, depending on the @a, may be way off the initial size.
How often is this true? A quick and dirty grep suggests at least 2 so perhaps worth retaining the old behaviour.
Can we take that into account? Maybe something like
#define opaque_struct_size(p, a, s) ((s) ? size_add(ALIGN(sizeof(*(p)), (a)), (s)): sizeof(*p))
Or do it at the call site below.
> + * > + * Returns: Number of bytes needed or SIZE_MAX on overflow. > + */ > +#define opaque_struct_size(p, a, s) size_add(ALIGN(sizeof(*(p)), (a)), (s)) > + > +#define opaque_struct_data(p, a) PTR_ALIGN((void *)((p) + 1), (a)) > + > /** > * iio_device_alloc() - allocate an iio_dev from a driver > * @parent: Parent device. > @@ -1618,19 +1636,13 @@ struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv) > struct iio_dev *indio_dev; > size_t alloc_size; > > - alloc_size = sizeof(struct iio_dev_opaque); > - if (sizeof_priv) { > - alloc_size = ALIGN(alloc_size, IIO_DMA_MINALIGN); > - alloc_size += sizeof_priv; > - } > - if (sizeof_priv) alloc_size = opaque_struct_size(iio_dev_opaque, IIO_DMA_MINALIGN, sizeof_priv); else alloc_size = sizeof(struct iio_dev_opaque);
> + alloc_size = opaque_struct_size(iio_dev_opaque, IIO_DMA_MINALIGN, sizeof_priv); > iio_dev_opaque = kzalloc(alloc_size, GFP_KERNEL); > if (!iio_dev_opaque) > return NULL; > > indio_dev = &iio_dev_opaque->indio_dev; > - indio_dev->priv = (char *)iio_dev_opaque + > - ALIGN(sizeof(struct iio_dev_opaque), IIO_DMA_MINALIGN); > + indio_dev->priv = opaque_struct_data(iio_dev_opaque, IIO_DMA_MINALIGN);
Would have been safer if original code set this to NULL if sizeof_priv == 0
A driver doing that should never have used iio_priv() but nicer if it was NULL rather than off the end of the allocation.
Jonathan
> > indio_dev->dev.parent = parent; > indio_dev->dev.type = &iio_device_type;
| |