Re: [PATCH v13 0/5] UFFD write-tracking migration/snapshots

[Andrey Gruzdev]

On 12.02.2021 19:11, Peter Xu wrote:

> On Fri, Feb 12, 2021 at 09:52:52AM +0100, David Hildenbrand wrote:
> > On 12.02.21 04:06, Peter Xu wrote:
> > > On Thu, Feb 11, 2021 at 10:09:58PM +0100, David Hildenbrand wrote:
> > > > The issue is when the discard happened before starting the snapshot. Write-protection won‘t work and the zeroed content won‘t be retained in the snapshot.
> > > >           
> > > I see what you mean now, and iiuc it will only be a problem if init_on_free=1.
> > > I think CONFIG_INIT_ON_FREE_DEFAULT_ON should be off for most distros, so the
> > >         
> > Yes, some distros seem to enable init_on_alloc instead. Looking at the
> > introducing commit 6471384af2a6 ("mm: security: introduce init_on_alloc=1
> > and init_on_free=1 boot options") there are security use cases and it might
> > become important with memory tagging.
> >
> > Note that in Linux, there was also the option to poison pages with 0,
> > removed via f289041ed4cf ("mm, page_poison: remove
> > CONFIG_PAGE_POISONING_ZERO"), available in some kernels that supported free
> > page reporting.
> >
> > It got removed and use cases got told to use init_on_free.
> >       
>     
I think we talk about init_on_free()/init_on_alloc() on guest side, right?
Still can't get how it relates to host's unpopulated pages..
 
Try to look from hardware side. Untouched SDRAM in hardware is required to contain zeroes somehow? No.
These 'trash' pages in migration stream are like never written physical memory pages, they are really
not needed in snapshot but they don't do any harm as well as there's no harm in that never-written physical
page is full of garbage.

Do these 'trash' pages in snapshot contain sensitive information not allowed to be accessed by the same VM?
I think no. Or we need a good example how it can be potentially exploited.

The only issue that I see is madvise(MADV_DONTNEED) for RAM blocks during snapshotting. And free page reporting
or memory balloon is secondary - the point is that UFFD_WP snapshot is incompatible with madvise(MADV_DONTNEED) on
hypervisor side. No matter which guest functionality can induce it.
> > > impact should be small, I think.  I thought about it, but indeed I didn't see a
> > > good way to fix this if without fixing the zero page copy for live snapshot.
> > >         
> > We should really document this (unexpected) behavior of snapshotting.
> > Otherwise, the next feature comes around that relies on pages that were
> > discarded to remain zeroed (I even have one in mind ;) ) and forgets to
> > disable snapshots.
> >       
> Agreed.  I'll see whether Andrey would have any idea to workaround this, or
> further comment.  Or I can draft a patch to document this next week (or unless
> Andrey would beat me to it :).
>
>     
Really better to document this specific behaviour but also clarify that the saved state remains
consistent and secure, off course if you agree with my arguments.

-- 
Andrey Gruzdev, Principal Engineer
Virtuozzo GmbH  +7-903-247-6397
                virtuzzo.com