[virtio-dev] Re: [PATCH v3] virtio-iommu: Rework the bypass feature

2021-10-31 Thread Eric Auger
Hi Jean,

On 10/22/21 2:12 PM, Jean-Philippe Brucker wrote:
> The VIRTIO_IOMMU_F_BYPASS feature is awkward to use and incomplete.
> Although it is implemented by QEMU, it is not supported by any driver as
> far as I know. Replace it with a new VIRTIO_IOMMU_F_BYPASS_CONFIG
> feature.
>
> Two features are missing from virtio-iommu:
>
> * The ability for an hypervisor to start the device in bypass mode. The
>   wording for VIRTIO_IOMMU_F_BYPASS is not clear enough to allow it at
>   the moment, because it only specifies the behavior after feature
>   negotiation.
>
> * The ability for a guest to set individual endpoints in bypass mode
>   when bypass is globally disabled. An OS should have the ability to
>   allow only endpoints it trusts to bypass the IOMMU, while keeping DMA
>   disabled for endpoints it isn't even aware of. At the moment this can
>   only be emulated by creating identity mappings.
>
> The VIRTIO_IOMMU_F_BYPASS_CONFIG feature adds a 'bypass' config field
> that allows to enable and disable bypass globally. It also adds a new
> flag for the ATTACH request.
>
> * The hypervisor can start the VM with bypass enabled or, if it knows
>   that the software stack supports it, disabled. The 'bypass' config
>   fields is initialized to 0 or 1. It is sticky and isn't affected by
>   device reset.
>
> * Generally the firmware won't have an IOMMU driver and will need to be
>   started in bypass mode, so the bootloader and kernel can be loaded
>   from storage endpoint.
>
>   For more security, the firmware could implement a minimal virtio-iommu
>   driver that reuses existing virtio support and only touches the config
>   space. It could enable PCI bus mastering in bridges only for the
>   endpoints that need it, enable global IOMMU bypass by flipping a bit,
>   then tear everything down before handing control over to the OS. This
>   prevents vulnerability windows where a malicious endpoint reprograms
>   the IOMMU while the OS is configuring it [1].
>
>   The isolation provided by vIOMMUs has mainly been used for securely
>   assigning endpoints to untrusted applications so far, while kernel DMA
>   bypasses the IOMMU. But we can expect boot security to become as
>   important in virtualization as it presently is on bare-metal systems,
>   where some devices are untrusted and must never be able to access
>   memory that wasn't assigned to them.
>
> * The OS can enable and disable bypass globally. It can then enable
>   bypass for individual endpoints by attaching them to bypass domains,
>   using the new VIRTIO_IOMMU_ATTACH_F_BYPASS flag. It can disable bypass
>   by attaching them to normal domains.
>
> [1] IOMMU protection against I/O attacks: a vulnerability and a proof of 
> concept
> Morgan, B., Alata, É., Nicomette, V. et al.
> https://link.springer.com/article/10.1186/s13173-017-0066-7
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/119
> Signed-off-by: Jean-Philippe Brucker 
> ---
>
> The virtio-iommu spec with colored diff is available at
> https://jpbrucker.net/virtio-iommu/spec-bypass/virtio-iommu-f-bypass-config-v3-diff.pdf
>
> v1: https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg07817.html
> v2: https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg07898.html
> v3:
> * Normative statement about device reset vs. system reset - the bypass
>   bit is sticky across device reset to avoid the vulnerability described
>   above, but restored on system reset (a term also used by the virtio
>   memory device).
> * Explain that the field bypass is in effect as long as the new feature
>   is offered, even when not accepted by the driver
> * Another clarification about the state of an endpoint after detach.
> ---
>  conformance.tex  |   1 -
>  virtio-iommu.tex | 111 ++-
>  2 files changed, 90 insertions(+), 22 deletions(-)
>
> diff --git a/conformance.tex b/conformance.tex
> index c2d7959..5c10ab9 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -504,7 +504,6 @@ \section{Conformance Targets}\label{sec:Conformance / 
> Conformance Targets}
>  \begin{itemize}
>  \item \ref{devicenormative:Device Types / IOMMU Device / Feature bits}
>  \item \ref{devicenormative:Device Types / IOMMU Device / Device 
> configuration layout}
> -\item \ref{devicenormative:Device Types / IOMMU Device / Device 
> Initialization}
>  \item \ref{devicenormative:Device Types / IOMMU Device / Device operations}
>  \item \ref{devicenormative:Device Types / IOMMU Device / Device operations / 
> ATTACH request}
>  \item \ref{devicenormative:Device Types / IOMMU Device / Device operations / 
> DETACH request}
> diff --git a/virtio-iommu.tex b/virtio-iommu.tex
> index 08b358a..f8cbe88 100644
> --- a/virtio-iommu.tex
> +++ b/virtio-iommu.tex
> @@ -59,14 +59,21 @@ \subsection{Feature bits}\label{sec:Device Types / IOMMU 
> Device / Feature bits}
>VIRTIO_IOMMU_F_MAP_UNMAP is supported.}
>  
>  \item[VIRTIO_IOMMU_F_BYPASS (3)]
> -  When n

[virtio-dev] RE: [PATCH v3] virtio-iommu: Rework the bypass feature

2021-11-02 Thread Tian, Kevin
> From: Jean-Philippe Brucker 
> Sent: Friday, October 22, 2021 8:12 PM
> 
> The VIRTIO_IOMMU_F_BYPASS feature is awkward to use and incomplete.
> Although it is implemented by QEMU, it is not supported by any driver as
> far as I know. Replace it with a new VIRTIO_IOMMU_F_BYPASS_CONFIG
> feature.
> 
> Two features are missing from virtio-iommu:
> 
> * The ability for an hypervisor to start the device in bypass mode. The
>   wording for VIRTIO_IOMMU_F_BYPASS is not clear enough to allow it at
>   the moment, because it only specifies the behavior after feature
>   negotiation.
> 
> * The ability for a guest to set individual endpoints in bypass mode
>   when bypass is globally disabled. An OS should have the ability to
>   allow only endpoints it trusts to bypass the IOMMU, while keeping DMA
>   disabled for endpoints it isn't even aware of. At the moment this can
>   only be emulated by creating identity mappings.
> 
> The VIRTIO_IOMMU_F_BYPASS_CONFIG feature adds a 'bypass' config field
> that allows to enable and disable bypass globally. It also adds a new
> flag for the ATTACH request.
> 
> * The hypervisor can start the VM with bypass enabled or, if it knows
>   that the software stack supports it, disabled. The 'bypass' config
>   fields is initialized to 0 or 1. It is sticky and isn't affected by
>   device reset.
> 
> * Generally the firmware won't have an IOMMU driver and will need to be
>   started in bypass mode, so the bootloader and kernel can be loaded
>   from storage endpoint.
> 
>   For more security, the firmware could implement a minimal virtio-iommu
>   driver that reuses existing virtio support and only touches the config
>   space. It could enable PCI bus mastering in bridges only for the
>   endpoints that need it, enable global IOMMU bypass by flipping a bit,
>   then tear everything down before handing control over to the OS. This
>   prevents vulnerability windows where a malicious endpoint reprograms
>   the IOMMU while the OS is configuring it [1].
> 
>   The isolation provided by vIOMMUs has mainly been used for securely
>   assigning endpoints to untrusted applications so far, while kernel DMA
>   bypasses the IOMMU. But we can expect boot security to become as
>   important in virtualization as it presently is on bare-metal systems,
>   where some devices are untrusted and must never be able to access
>   memory that wasn't assigned to them.
> 
> * The OS can enable and disable bypass globally. It can then enable
>   bypass for individual endpoints by attaching them to bypass domains,
>   using the new VIRTIO_IOMMU_ATTACH_F_BYPASS flag. It can disable
> bypass
>   by attaching them to normal domains.
> 
> [1] IOMMU protection against I/O attacks: a vulnerability and a proof of
> concept
> Morgan, B., Alata, É., Nicomette, V. et al.
> https://link.springer.com/article/10.1186/s13173-017-0066-7
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/119
> Signed-off-by: Jean-Philippe Brucker 

Reviewed-by: Kevin Tian 

> ---
> 
> The virtio-iommu spec with colored diff is available at
> https://jpbrucker.net/virtio-iommu/spec-bypass/virtio-iommu-f-bypass-
> config-v3-diff.pdf
> 
> v1: https://www.mail-archive.com/virtio-dev@lists.oasis-
> open.org/msg07817.html
> v2: https://www.mail-archive.com/virtio-dev@lists.oasis-
> open.org/msg07898.html
> v3:
> * Normative statement about device reset vs. system reset - the bypass
>   bit is sticky across device reset to avoid the vulnerability described
>   above, but restored on system reset (a term also used by the virtio
>   memory device).
> * Explain that the field bypass is in effect as long as the new feature
>   is offered, even when not accepted by the driver
> * Another clarification about the state of an endpoint after detach.
> ---
>  conformance.tex  |   1 -
>  virtio-iommu.tex | 111 ++-
>  2 files changed, 90 insertions(+), 22 deletions(-)
> 
> diff --git a/conformance.tex b/conformance.tex
> index c2d7959..5c10ab9 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -504,7 +504,6 @@ \section{Conformance
> Targets}\label{sec:Conformance / Conformance Targets}
>  \begin{itemize}
>  \item \ref{devicenormative:Device Types / IOMMU Device / Feature bits}
>  \item \ref{devicenormative:Device Types / IOMMU Device / Device
> configuration layout}
> -\item \ref{devicenormative:Device Types / IOMMU Device / Device
> Initialization}
>  \item \ref{devicenormative:Device Types / IOMMU Device / Device
> operations}
>  \item \ref{devicenormative:Device Types / IOMMU Device / Device
> operations / ATTACH request}
>  \item \ref{devicenormative:Device Types / IOMMU Device / Device
> operations / DETACH request}
> diff --git a/virtio-iommu.tex b/virtio-iommu.tex
> index 08b358a..f8cbe88 100644
> --- a/virtio-iommu.tex
> +++ b/virtio-iommu.tex
> @@ -59,14 +59,21 @@ \subsection{Feature bits}\label{sec:Device Types /
> IOMMU Device / Feature bits}
>VIRTIO_IOMMU_F_MAP_UNMAP is s

[virtio-dev] Re: [PATCH v3] virtio-iommu: Rework the bypass feature

2021-11-03 Thread Cornelia Huck
On Fri, Oct 22 2021, Jean-Philippe Brucker  wrote:

> The VIRTIO_IOMMU_F_BYPASS feature is awkward to use and incomplete.
> Although it is implemented by QEMU, it is not supported by any driver as
> far as I know. Replace it with a new VIRTIO_IOMMU_F_BYPASS_CONFIG
> feature.
>
> Two features are missing from virtio-iommu:
>
> * The ability for an hypervisor to start the device in bypass mode. The
>   wording for VIRTIO_IOMMU_F_BYPASS is not clear enough to allow it at
>   the moment, because it only specifies the behavior after feature
>   negotiation.
>
> * The ability for a guest to set individual endpoints in bypass mode
>   when bypass is globally disabled. An OS should have the ability to
>   allow only endpoints it trusts to bypass the IOMMU, while keeping DMA
>   disabled for endpoints it isn't even aware of. At the moment this can
>   only be emulated by creating identity mappings.
>
> The VIRTIO_IOMMU_F_BYPASS_CONFIG feature adds a 'bypass' config field
> that allows to enable and disable bypass globally. It also adds a new
> flag for the ATTACH request.
>
> * The hypervisor can start the VM with bypass enabled or, if it knows
>   that the software stack supports it, disabled. The 'bypass' config
>   fields is initialized to 0 or 1. It is sticky and isn't affected by
>   device reset.
>
> * Generally the firmware won't have an IOMMU driver and will need to be
>   started in bypass mode, so the bootloader and kernel can be loaded
>   from storage endpoint.
>
>   For more security, the firmware could implement a minimal virtio-iommu
>   driver that reuses existing virtio support and only touches the config
>   space. It could enable PCI bus mastering in bridges only for the
>   endpoints that need it, enable global IOMMU bypass by flipping a bit,
>   then tear everything down before handing control over to the OS. This
>   prevents vulnerability windows where a malicious endpoint reprograms
>   the IOMMU while the OS is configuring it [1].
>
>   The isolation provided by vIOMMUs has mainly been used for securely
>   assigning endpoints to untrusted applications so far, while kernel DMA
>   bypasses the IOMMU. But we can expect boot security to become as
>   important in virtualization as it presently is on bare-metal systems,
>   where some devices are untrusted and must never be able to access
>   memory that wasn't assigned to them.
>
> * The OS can enable and disable bypass globally. It can then enable
>   bypass for individual endpoints by attaching them to bypass domains,
>   using the new VIRTIO_IOMMU_ATTACH_F_BYPASS flag. It can disable bypass
>   by attaching them to normal domains.
>
> [1] IOMMU protection against I/O attacks: a vulnerability and a proof of 
> concept
> Morgan, B., Alata, É., Nicomette, V. et al.
> https://link.springer.com/article/10.1186/s13173-017-0066-7
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/119
> Signed-off-by: Jean-Philippe Brucker 
> ---
>
> The virtio-iommu spec with colored diff is available at
> https://jpbrucker.net/virtio-iommu/spec-bypass/virtio-iommu-f-bypass-config-v3-diff.pdf
>
> v1: https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg07817.html
> v2: https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg07898.html
> v3:
> * Normative statement about device reset vs. system reset - the bypass
>   bit is sticky across device reset to avoid the vulnerability described
>   above, but restored on system reset (a term also used by the virtio
>   memory device).
> * Explain that the field bypass is in effect as long as the new feature
>   is offered, even when not accepted by the driver
> * Another clarification about the state of an endpoint after detach.
> ---
>  conformance.tex  |   1 -
>  virtio-iommu.tex | 111 ++-
>  2 files changed, 90 insertions(+), 22 deletions(-)

Now looks good to me.

Michael, do you think we should start voting?


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org