On 10/20/2018 07:10 PM, Cornelia Huck wrote: > On Thu, 18 Oct 2018 15:58:48 +0200 > Halil Pasic <pasic@xxxxxxxxxxxxx> wrote: > >> On 10/18/2018 09:51 AM, Pierre Morel wrote: >>> There is a risk that the mediated device is used before all the >>> data are initialized if it is register too early. >> >> How big is the risk? Is this a bug fix (if it is do we need cc stable) >> or readability improvement? > > This looks like a bug fix to me, but not sure if it is stable material. > I wanted to apply as-is. > Isn't atomic_set(&private->avail, 1); sufficient to ensure no mdev is created on top of the parent dev unless private->io_work is already initialized? (->avail an ->state are zero initialized, which for the latter means VFIO_CCW_STATE_NOT_OPER.) Pierre states in the cover letter that this one is a bugfix. But I don't quite get it. If it is an exploitable bug, I think it should be stable material, or? Regardless, I would prefer a better commit message and title. For instance 'if it is register too early' does not seem grammatical to me, and the 'register' is also misleading: I guess, it is not about vfio_ccw_mdev_reg() but the register triggered by mdev create. But no strong feelings here: the patch, if nothing else, contributes to readability, thus it is useful. BTW I do remember not being confident about the synchronization in vfio-ccw, but was hoping all this gets cleared up when the new functions (clear, halt, etc) get implement. Regards, Halil >> >> I would prefer a commit message that clearly answers these questions. >> >> Otherwise I don't have a strong opinion. >> >> Regards, >> Halil >> >>> >>> Let's register the mediated device when all the data structures >>> which could be used are initialized. >>> >>> Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx> >> >
