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 <m...@redhat.com> > > > > 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 <dgilb...@redhat.com> > > > > > > Signed-off-by: Stefano Garzarella <sgarz...@redhat.com> > > > > > > --- > > > > > > 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