All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pierre Morel <pmorel@linux.ibm.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: linux-kernel@vger.kernel.org, pasic@linux.ibm.com,
	borntraeger@de.ibm.com, frankja@linux.ibm.com,
	jasowang@redhat.com, cohuck@redhat.com, kvm@vger.kernel.org,
	linux-s390@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	thomas.lendacky@amd.com, david@gibson.dropbear.id.au,
	linuxram@us.ibm.com, heiko.carstens@de.ibm.com,
	gor@linux.ibm.com
Subject: Re: [PATCH v3 1/1] s390: virtio: let arch accept devices without IOMMU feature
Date: Mon, 29 Jun 2020 18:48:28 +0200	[thread overview]
Message-ID: <66f808f2-5dd9-9127-d0e8-6bafbf13fc62@linux.ibm.com> (raw)
In-Reply-To: <20200629115952-mutt-send-email-mst@kernel.org>



On 2020-06-29 18:09, Michael S. Tsirkin wrote:
> On Wed, Jun 17, 2020 at 12:43:57PM +0200, Pierre Morel wrote:
>> An architecture protecting the guest memory against unauthorized host
>> access may want to enforce VIRTIO I/O device protection through the
>> use of VIRTIO_F_IOMMU_PLATFORM.
>> Let's give a chance to the architecture to accept or not devices
>> without VIRTIO_F_IOMMU_PLATFORM.
> 
> I agree it's a bit misleading. Protection is enforced by memory
> encryption, you can't trust the hypervisor to report the bit correctly
> so using that as a securoty measure would be pointless.
> The real gain here is that broken configs are easier to
> debug.
> 
> Here's an attempt at a better description:
> 
> 	On some architectures, guest knows that VIRTIO_F_IOMMU_PLATFORM is
> 	required for virtio to function: e.g. this is the case on s390 protected
> 	virt guests, since otherwise guest passes encrypted guest memory to devices,
> 	which the device can't read. Without VIRTIO_F_IOMMU_PLATFORM the
> 	result is that affected memory (or even a whole page containing
> 	it is corrupted). Detect and fail probe instead - that is easier
> 	to debug.

Thanks indeed better aside from the "encrypted guest memory": the 
mechanism used to avoid the access to the guest memory from the host by 
s390 is not encryption but a hardware feature denying the general host 
access and allowing pieces of memory to be shared between guest and host.
As a consequence the data read from memory is not corrupted but not read 
at all and the read error kills the hypervizor with a SIGSEGV.


> 
> however, now that we have described what it is (hypervisor
> misconfiguration) I ask a question: can we be sure this will never
> ever work? E.g. what if some future hypervisor gains ability to
> access the protected guest memory in some abstractly secure manner?

The goal of the s390 PV feature is to avoid this possibility so I don't 
think so; however, there is a possibility that some hardware VIRTIO 
device gain access to the guest's protected memory, even such device 
does not exist yet.

At the moment such device exists we will need a driver for it, at least 
to enable the feature and apply policies, it is also one of the reasons 
why a hook to the architecture is interesting.

> We are blocking this here, and it's hard to predict the future,
> and a broken hypervisor can always find ways to crash the guest ...

yes, this is also something to fix on the hypervizor side, Halil is 
working on it.

> 
> IMHO it would be safer to just print a warning.
> What do you think?

Sadly, putting a warning may not help as qemu is killed if it accesses 
the protected memory.
Also note that the crash occurs not only on start but also on hotplug.

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

WARNING: multiple messages have this Message-ID (diff)
From: Pierre Morel <pmorel@linux.ibm.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: gor@linux.ibm.com, linux-s390@vger.kernel.org,
	frankja@linux.ibm.com, kvm@vger.kernel.org,
	thomas.lendacky@amd.com, heiko.carstens@de.ibm.com,
	cohuck@redhat.com, linuxram@us.ibm.com,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org, pasic@linux.ibm.com,
	borntraeger@de.ibm.com, david@gibson.dropbear.id.au
Subject: Re: [PATCH v3 1/1] s390: virtio: let arch accept devices without IOMMU feature
Date: Mon, 29 Jun 2020 18:48:28 +0200	[thread overview]
Message-ID: <66f808f2-5dd9-9127-d0e8-6bafbf13fc62@linux.ibm.com> (raw)
In-Reply-To: <20200629115952-mutt-send-email-mst@kernel.org>



On 2020-06-29 18:09, Michael S. Tsirkin wrote:
> On Wed, Jun 17, 2020 at 12:43:57PM +0200, Pierre Morel wrote:
>> An architecture protecting the guest memory against unauthorized host
>> access may want to enforce VIRTIO I/O device protection through the
>> use of VIRTIO_F_IOMMU_PLATFORM.
>> Let's give a chance to the architecture to accept or not devices
>> without VIRTIO_F_IOMMU_PLATFORM.
> 
> I agree it's a bit misleading. Protection is enforced by memory
> encryption, you can't trust the hypervisor to report the bit correctly
> so using that as a securoty measure would be pointless.
> The real gain here is that broken configs are easier to
> debug.
> 
> Here's an attempt at a better description:
> 
> 	On some architectures, guest knows that VIRTIO_F_IOMMU_PLATFORM is
> 	required for virtio to function: e.g. this is the case on s390 protected
> 	virt guests, since otherwise guest passes encrypted guest memory to devices,
> 	which the device can't read. Without VIRTIO_F_IOMMU_PLATFORM the
> 	result is that affected memory (or even a whole page containing
> 	it is corrupted). Detect and fail probe instead - that is easier
> 	to debug.

Thanks indeed better aside from the "encrypted guest memory": the 
mechanism used to avoid the access to the guest memory from the host by 
s390 is not encryption but a hardware feature denying the general host 
access and allowing pieces of memory to be shared between guest and host.
As a consequence the data read from memory is not corrupted but not read 
at all and the read error kills the hypervizor with a SIGSEGV.


> 
> however, now that we have described what it is (hypervisor
> misconfiguration) I ask a question: can we be sure this will never
> ever work? E.g. what if some future hypervisor gains ability to
> access the protected guest memory in some abstractly secure manner?

The goal of the s390 PV feature is to avoid this possibility so I don't 
think so; however, there is a possibility that some hardware VIRTIO 
device gain access to the guest's protected memory, even such device 
does not exist yet.

At the moment such device exists we will need a driver for it, at least 
to enable the feature and apply policies, it is also one of the reasons 
why a hook to the architecture is interesting.

> We are blocking this here, and it's hard to predict the future,
> and a broken hypervisor can always find ways to crash the guest ...

yes, this is also something to fix on the hypervizor side, Halil is 
working on it.

> 
> IMHO it would be safer to just print a warning.
> What do you think?

Sadly, putting a warning may not help as qemu is killed if it accesses 
the protected memory.
Also note that the crash occurs not only on start but also on hotplug.

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

  reply	other threads:[~2020-06-29 18:55 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-17 10:43 [PATCH v3 0/1] s390: virtio: let arch choose to accept devices without IOMMU feature Pierre Morel
2020-06-17 10:43 ` [PATCH v3 1/1] s390: virtio: let arch " Pierre Morel
2020-06-17 11:22   ` Heiko Carstens
2020-06-17 11:59     ` Pierre Morel
2020-06-17 13:36   ` Tom Lendacky
2020-06-17 14:12     ` Pierre Morel
2020-06-17 22:29   ` Halil Pasic
2020-06-19  9:20     ` Cornelia Huck
2020-06-19 12:02       ` Halil Pasic
2020-06-19 12:02         ` Halil Pasic
2020-06-29 13:15         ` Pierre Morel
2020-06-29 13:15           ` Pierre Morel
2020-06-29 13:14       ` Pierre Morel
2020-06-29 13:14         ` Pierre Morel
2020-06-29 13:44         ` Cornelia Huck
2020-06-29 13:44           ` Cornelia Huck
2020-06-29 16:10           ` Pierre Morel
2020-06-29 16:10             ` Pierre Morel
2020-06-29 13:21     ` Pierre Morel
2020-06-29 13:21       ` Pierre Morel
2020-06-29 15:57   ` Michael S. Tsirkin
2020-06-29 15:57     ` Michael S. Tsirkin
2020-06-29 16:05     ` Cornelia Huck
2020-06-29 16:05       ` Cornelia Huck
2020-07-02 13:03       ` Pierre Morel
2020-07-06 13:37         ` Pierre Morel
2020-07-06 14:33           ` Cornelia Huck
2020-07-06 15:01             ` Pierre Morel
2020-06-29 16:09     ` Pierre Morel
2020-06-29 16:09       ` Pierre Morel
2020-06-29 16:09   ` Michael S. Tsirkin
2020-06-29 16:09     ` Michael S. Tsirkin
2020-06-29 16:48     ` Pierre Morel [this message]
2020-06-29 16:48       ` Pierre Morel
2020-06-29 21:18       ` Michael S. Tsirkin
2020-06-29 21:18         ` Michael S. Tsirkin
2020-06-30  7:08         ` Cornelia Huck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=66f808f2-5dd9-9127-d0e8-6bafbf13fc62@linux.ibm.com \
    --to=pmorel@linux.ibm.com \
    --cc=borntraeger@de.ibm.com \
    --cc=cohuck@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=frankja@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=jasowang@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linuxram@us.ibm.com \
    --cc=mst@redhat.com \
    --cc=pasic@linux.ibm.com \
    --cc=thomas.lendacky@amd.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.