linux-kernel - Re: [PATCH v4 2/7] iio: core: wrap IIO device into an iio_dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200721094643.GA1598380@eriador.lumag.spb.ru>
Date:   Tue, 21 Jul 2020 12:46:43 +0300
From:   Dmitry Baryshkov <dbaryshkov@...il.com>
To:     Alexandru Ardelean <alexandru.ardelean@...log.com>
Cc:     <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <jic23@...nel.org>, <lars@...afoo.de>, <pmeerw@...erw.net>,
        <knaack.h@....de>, Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH v4 2/7] iio: core: wrap IIO device into an iio_dev_opaque
 object

Hello,

On Tue, Jun 30, 2020 at 07:57:03AM +0300, Alexandru Ardelean wrote:
> There are plenty of bad designs we want to discourage or not have to review
> manually usually about accessing private (marked as [INTERN]) fields of
> 'struct iio_dev'.
> 
> Sometimes users copy drivers that are not always the best examples.
> 
> A better idea is to hide those fields into the framework.
> For 'struct iio_dev' this is a 'struct iio_dev_opaque' which wraps a public
> 'struct iio_dev' object.
> 
> In the next series, some fields will be moved to this new struct, each with
> it's own rework.

[skipped]

> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@...log.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> ---
>  drivers/iio/industrialio-core.c | 19 +++++++++++++------
>  include/linux/iio/iio-opaque.h  | 17 +++++++++++++++++
>  include/linux/iio/iio.h         |  6 +++++-
>  3 files changed, 35 insertions(+), 7 deletions(-)
>  create mode 100644 include/linux/iio/iio-opaque.h
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 75661661aaba..33e2953cf021 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c

[skipped]

> @@ -1506,11 +1510,14 @@ struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv)
>  	/* ensure 32-byte alignment of whole construct ? */
>  	alloc_size += IIO_ALIGN - 1;
>  
> -	dev = kzalloc(alloc_size, GFP_KERNEL);
> -	if (!dev)
> +	iio_dev_opaque = kzalloc(alloc_size, GFP_KERNEL);
> +	if (!iio_dev_opaque)
>  		return NULL;
>  
> -	dev->dev.parent = parent;

This chunk (together with 8cb631ccbb1952b6422917f2ed16f760d84a762e and
d3be83244c7dfe686d23f1c0bac75915587fc044) break devicetree bindings of
IIO clients. After these changes there are no links between devicetree
nodes and corresponding IIO channels. I'd kindly ask to restore this
dev->dev.parent assignment (which restores of node binding).

> +	dev = &iio_dev_opaque->indio_dev;
> +	dev->priv = (char *)iio_dev_opaque +
> +		ALIGN(sizeof(struct iio_dev_opaque), IIO_ALIGN);
> +
>  	dev->dev.groups = dev->groups;
>  	dev->dev.type = &iio_device_type;
>  	dev->dev.bus = &iio_bus_type;

-- 
With best wishes
Dmitry