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] [day] [month] [year] [list]

Message-ID: <CA+U=DsqLAnXG0BZvXw5=_rUS_N-1qSs2Xxu7-x2qA2HH7pPyVA@mail.gmail.com>
Date:   Tue, 21 Jul 2020 13:00:11 +0300
From:   Alexandru Ardelean <ardeleanalex@...il.com>
To:     Dmitry Baryshkov <dbaryshkov@...il.com>
Cc:     Alexandru Ardelean <alexandru.ardelean@...log.com>,
        linux-iio@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Hartmut Knaack <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

On Tue, Jul 21, 2020 at 12:47 PM Dmitry Baryshkov <dbaryshkov@...il.com> wrote:
>
> 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).
>

Ack.
Will send a fix.
Thanks for catching this.
Apologies for the breakage.
It seems that I was tripping quite a bit with too many patchsets.


> > +     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