Re: virtio-balloon: VIRTIO_BALLOON_F_PAGE_POISON discussion

Re: virtio-balloon: VIRTIO_BALLOON_F_PAGE_POISON discussion

[Wei Wang]

On 11/07/2018 11:27 PM, Michael S. Tsirkin wrote:

+ LKML

> On Wed, Nov 07, 2018 at 02:29:02PM +0000, Wang, Wei W wrote:
>> Hi Michael,
>>
>>   
>>
>> Thanks again for reviewing so many versions of patches, and I learnt a lot from
>> your comments.
>>
>>   
>>
>> While I’m writing the virtio-balloon spec patches, I’m thinking probably we
>> don’t need VIRTIO_BALLOON_F_PAGE_POISON to limit
>> VIRTIO_BALLOON_F_FREE_PAGE_HINT, because now the guest frees the allocated
>> pages after the migration is done (that is, the skipped free pages will be
>> poisoned when the guest is already on the destination machine).
> The concern was this:
>
> guest poisons the page by writing a non-0 pattern there
> guest sends page to host
> VM is migrated, page is unmapped
> guest reads page, zero page is mapped

Not sure about this one: I think guest wouldn't read the page,
since they are held by balloon (balloon itself will also
not read it, the page just stays on a list waiting to be freed).
Please see the below example.

> guest sees 0 in page and detects it as use after free

  - balloon collects (i.e. alloc) a free page X (now it
    has 0xaa poison value) and reports X to host to be skipped in
    migration;
  -  Now VM is migrated to the destination, and on the destination
     side, X is not mapped initially.
  -  Nobody will access X since it has been taken by balloon
     and stays on a list waiting to be freed. So the first chance
     that will get X mapped will be the moment that balloon
     returns X to mm via free(), as free() writes the
     poison value to X.


Best,
Wei

Re: virtio-balloon: VIRTIO_BALLOON_F_PAGE_POISON discussion

[Michael S. Tsirkin]

On Thu, Nov 08, 2018 at 10:49:20AM +0800, Wei Wang wrote:
> On 11/07/2018 11:27 PM, Michael S. Tsirkin wrote:
> 
> + LKML
> 
> > On Wed, Nov 07, 2018 at 02:29:02PM +0000, Wang, Wei W wrote:
> > > Hi Michael,
> > > 
> > > 
> > > Thanks again for reviewing so many versions of patches, and I learnt a lot from
> > > your comments.
> > > 
> > > 
> > > While I’m writing the virtio-balloon spec patches, I’m thinking probably we
> > > don’t need VIRTIO_BALLOON_F_PAGE_POISON to limit
> > > VIRTIO_BALLOON_F_FREE_PAGE_HINT, because now the guest frees the allocated
> > > pages after the migration is done (that is, the skipped free pages will be
> > > poisoned when the guest is already on the destination machine).
> > The concern was this:
> > 
> > guest poisons the page by writing a non-0 pattern there
> > guest sends page to host
> > VM is migrated, page is unmapped
> > guest reads page, zero page is mapped
> 
> Not sure about this one: I think guest wouldn't read the page,
> since they are held by balloon (balloon itself will also
> not read it, the page just stays on a list waiting to be freed).
> Please see the below example.
> 
> > guest sees 0 in page and detects it as use after free
> 
>  - balloon collects (i.e. alloc) a free page X (now it
>    has 0xaa poison value) and reports X to host to be skipped in
>    migration;
>  -  Now VM is migrated to the destination, and on the destination
>     side, X is not mapped initially.
>  -  Nobody will access X since it has been taken by balloon
>     and stays on a list waiting to be freed. So the first chance
>     that will get X mapped will be the moment that balloon
>     returns X to mm via free(), as free() writes the
>     poison value to X.
> 
> 
> Best,
> Wei


Oh I see, that was with the previous design where we bypassed alloc.
I think you are right, but better stress-test it.

-- 
MST

Re: virtio-balloon: VIRTIO_BALLOON_F_PAGE_POISON discussion

[Wei Wang]

On 11/08/2018 10:50 AM, Michael S. Tsirkin wrote:
> On Thu, Nov 08, 2018 at 10:49:20AM +0800, Wei Wang wrote:
>> On 11/07/2018 11:27 PM, Michael S. Tsirkin wrote:
>>
>> + LKML
>>
>>> On Wed, Nov 07, 2018 at 02:29:02PM +0000, Wang, Wei W wrote:
>>>> Hi Michael,
>>>>
>>>>
>>>> Thanks again for reviewing so many versions of patches, and I learnt a lot from
>>>> your comments.
>>>>
>>>>
>>>> While I’m writing the virtio-balloon spec patches, I’m thinking probably we
>>>> don’t need VIRTIO_BALLOON_F_PAGE_POISON to limit
>>>> VIRTIO_BALLOON_F_FREE_PAGE_HINT, because now the guest frees the allocated
>>>> pages after the migration is done (that is, the skipped free pages will be
>>>> poisoned when the guest is already on the destination machine).
>>> The concern was this:
>>>
>>> guest poisons the page by writing a non-0 pattern there
>>> guest sends page to host
>>> VM is migrated, page is unmapped
>>> guest reads page, zero page is mapped
>> Not sure about this one: I think guest wouldn't read the page,
>> since they are held by balloon (balloon itself will also
>> not read it, the page just stays on a list waiting to be freed).
>> Please see the below example.
>>
>>> guest sees 0 in page and detects it as use after free
>>   - balloon collects (i.e. alloc) a free page X (now it
>>     has 0xaa poison value) and reports X to host to be skipped in
>>     migration;
>>   -  Now VM is migrated to the destination, and on the destination
>>      side, X is not mapped initially.
>>   -  Nobody will access X since it has been taken by balloon
>>      and stays on a list waiting to be freed. So the first chance
>>      that will get X mapped will be the moment that balloon
>>      returns X to mm via free(), as free() writes the
>>      poison value to X.
>>
>>
>> Best,
>> Wei
>
> Oh I see, that was with the previous design where we bypassed alloc.
> I think you are right, but better stress-test it.
>

Sure, will do.

Best,
Wei