qemu-block
[Top][All Lists]
Advanced

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

Re: [Qemu-block] [PATCH v2 2/5] virtio-blk: add "discard-wzeroes" boolea


From: Stefano Garzarella
Subject: Re: [Qemu-block] [PATCH v2 2/5] virtio-blk: add "discard-wzeroes" boolean property
Date: Mon, 4 Feb 2019 16:38:24 +0100
User-agent: NeoMutt/20180716

On Mon, Feb 04, 2019 at 08:37:28AM -0500, Michael S. Tsirkin wrote:
> On Mon, Feb 04, 2019 at 11:16:14AM +0100, Stefano Garzarella wrote:
> > On Mon, Feb 04, 2019 at 11:33:07AM +0800, Stefan Hajnoczi wrote:
> > > 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...
> > 
> > Many thanks for the explanation!
> > 
> > I'll wait to send the v3 in order to understand if Michael agrees to
> > leave discard feature enabled to default.
> > 
> > Thanks,
> > Stefano
> 
> OK. Maybe mention the above in the commit log.
> 

Ok, I'll do it!

Thanks,
Stefano



reply via email to

[Prev in Thread] Current Thread [Next in Thread]