Re: [PATCH v1 2/2] vfio: ccw: Register mediated device once all structures are initialized

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux