linux-kernel - Re: [PATCH v2 06/10] vfio: ccw: Make FSM functions atomic

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:   Tue, 5 Jun 2018 17:15:40 +0200
From:   Cornelia Huck <cohuck@...hat.com>
To:     Pierre Morel <pmorel@...ux.ibm.com>
Cc:     Pierre Morel <pmorel@...ux.vnet.ibm.com>, pasic@...ux.vnet.ibm.com,
        bjsdjshi@...ux.vnet.ibm.com, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v2 06/10] vfio: ccw: Make FSM functions atomic

On Tue, 5 Jun 2018 16:21:03 +0200
Pierre Morel <pmorel@...ux.ibm.com> wrote:

> On 05/06/2018 15:35, Cornelia Huck wrote:
> > On Tue, 5 Jun 2018 15:10:11 +0200
> > Pierre Morel <pmorel@...ux.ibm.com> wrote:
> >  
> >> On 05/06/2018 13:38, Cornelia Huck wrote:  
> >>> On Fri, 25 May 2018 12:21:14 +0200
> >>> Pierre Morel <pmorel@...ux.vnet.ibm.com> wrote:
> >>>     
> >>>> We use mutex around the FSM function call to make the FSM
> >>>> event handling and state change atomic.  
> >>> I'm still not really clear as to what this mutex is supposed to
> >>> serialize:
> >>>
> >>> - Modification of the state?
> >>> - Any calls in the state machine?
> >>> - A combination? (That would imply that we only deal with the state in
> >>>     the state machine.)  
> >> yes to all  
> > But wouldn't that imply that you need to either take the mutex if you
> > do something dependent on the state, or fire an event in that case?  
> 
> Yes, if it is not I forgot something important (like I did in patch 10)
> vfio_ccw_fsm_event(private, event) takes the mutex on firering an event.
> 
> I have some cases where I do not respect this.
> This is false and I must handle this with a new private variable,
> this is where I test the state after having fired an event.
> I will need to change this, in quiesce, reset, probe and open (others?).

But isn't that inherently racy? Even if you check the return code from
the state machine change, it might have changed in the meantime again.