Re: [Qemu-devel] [PATCH v2 2/5] virtio-blk: add "discard-wzeroes" boolean property

[Stefan Hajnoczi]

On Fri, Feb 01, 2019 at 06:18:52PM +0100, Stefano Garzarella wrote:
> On Fri, Feb 1, 2019 at 4:17 PM Michael S. Tsirkin <address@hidden> wrote:
> > On Thu, Jan 31, 2019 at 04:19:11PM +0100, Stefano Garzarella wrote:
> > > In order to avoid migration issues, we enable DISCARD and
> > > WRITE ZEROES features only for machine type >= 4.0
> > >
> > > Suggested-by: Dr. David Alan Gilbert <address@hidden>
> > > Signed-off-by: Stefano Garzarella <address@hidden>
> > > ---
> > >  hw/block/virtio-blk.c          | 2 ++
> > >  hw/core/machine.c              | 1 +
> > >  include/hw/virtio/virtio-blk.h | 1 +
> > >  3 files changed, 4 insertions(+)
> > >
> > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > > index 8a6754d9a2..542ec52536 100644
> > > --- a/hw/block/virtio-blk.c
> > > +++ b/hw/block/virtio-blk.c
> > > @@ -1026,6 +1026,8 @@ static Property virtio_blk_properties[] = {
> > >      DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
> > >      DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, 
> > > TYPE_IOTHREAD,
> > >                       IOThread *),
> > > +    DEFINE_PROP_BIT("discard-wzeroes", VirtIOBlock, 
> > > conf.discard_wzeroes, 0,
> > > +                     true),
> > >      DEFINE_PROP_END_OF_LIST(),
> > >  };
> > >
> >
> > Thinking about it, are there security implications for discard?
> > Should we make it default false?
> 
> Hi Michael,
> 
> I'm not completely sure but if the guest can write on a specific sector,
> discard or write_zeroes operations should not have a security implication.
> 
> Do I miss something?

Recently page cache attacks have been discussed in the Linux community:
https://arxiv.org/pdf/1901.01161.pdf

I guess the scenario Michael is thinking about involves either -drive
cache.direct=off (including cache=writeback or cache=writethrough) or
maybe a timing side-channel in the storage appliance.

The discard operation may allow a guest to evict the cache, an important
primitive for page cache attacks.

Most of the time disk images are not shared between guests at all.
Therefore the discard operation cannot be used to learn information
about other guests.

Let's focus on shared disk images: shared disk images are either
read-only (then discard isn't allowed anyway) or they are shared
writable (but this already implies a trust relationship between the
guests).

My opinion is that discard is safe because virtualization use cases are
quite different from the attacks possible with shared library files
between userspace processes.  I'm curious if anyone has figured out a
realistic scenario where it does matter though...

Stefan

signature.asc
Description: PGP signature