linux-kernel - Re: [RFC PATCH v1] iio: trigger: move trig->owner init to trigger allocate() stage

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]

Date:   Mon, 6 Jun 2022 16:13:02 +0100
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Dmitry Rokosov <DDRokosov@...rdevices.ru>
CC:     Jonathan Cameron <jic23@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "lars@...afoo.de" <lars@...afoo.de>,
        "andy.shevchenko@...il.com" <andy.shevchenko@...il.com>,
        "noname.nuno@...il.com" <noname.nuno@...il.com>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        kernel <kernel@...rdevices.ru>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v1] iio: trigger: move trig->owner init to trigger
 allocate() stage

On Mon, 6 Jun 2022 11:37:42 +0000
Dmitry Rokosov <DDRokosov@...rdevices.ru> wrote:

> Hello Jonathan,
> 
> Thank you for comments. I have a several questions about the flow,
> please find them below.
> 
> On Sat, Jun 04, 2022 at 02:59:55PM +0100, Jonathan Cameron wrote:
> > On Wed, 1 Jun 2022 17:48:32 +0000
> > Dmitry Rokosov <DDRokosov@...rdevices.ru> wrote:
> >   
> > > To provide a new IIO trigger to the IIO core, usually driver executes the
> > > following pipeline: allocate()/register()/get(). Before, IIO core assigned
> > > trig->owner as a pointer to the module which registered this trigger at
> > > the register() stage. But actually the trigger object is owned by the
> > > module earlier, on the allocate() stage, when trigger object is
> > > successfully allocated for the driver.
> > > 
> > > This patch moves trig->owner initialization from register()
> > > stage of trigger initialization pipeline to allocate() stage to
> > > eliminate all misunderstandings and time gaps between trigger object
> > > creation and owner acquiring.
> > > 
> > > Signed-off-by: Dmitry Rokosov <ddrokosov@...rdevices.ru>  
> > 
> > Hi Dmitry,
> > 
> > I 'think' this is fine, but its in the high risk category that I'd like
> > to keep it on list for a few weeks before applying.
> >   
> 
> Could you please explain what it means? Do you have some testing branch
> with such dangerous patches or do we need just to wait other developers
> for more points of view? Thanks in advance.

The second - so far I haven't applied it anywhere.

> 
> > Note I'm still keen that in general we keep the flow such that
> > we do allocate()/register()/get() as there is no guarantee that the get()
> > will never do anything that requires the trigger to be registered, even
> > though that is true today.  Which is another way of saying I'm still
> > keen we fix up any cases that sneak in after your fix up set dealt with
> > the current ones.  
> 
> I fully agree with you. I suppose to resolve such a problem we need to
> have some indicators that the trigger is already registered or not.
> From my point of view, trig->list entry fits well to answer this question.
> Trigger is added to the global IIO triggers list during register()
> execution, so we can just check that entry is not empty to make sure that
> trigger is registered.
> 
> I've sent a v2 patch version, where I use trig->list entry empty status to
> warn it:
> 
> https://lore.kernel.org/linux-iio/20220606111316.19265-1-ddrokosov@sberdevices.ru/

Great!

Jonathan

> 
> > 
> > Thanks for following up on this!
> > 
> > Jonathan
> >   
>