[RFC] virtio: document virtio hardening status and TODO

Message ID 20221014042037.23639-1-jasowang@redhat.com
State New
Headers
Series [RFC] virtio: document virtio hardening status and TODO |

Commit Message

Jason Wang Oct. 14, 2022, 4:20 a.m. UTC
  This patch summarizes the status of hardening and TODO of hardening
virtio core and drivers.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 Documentation/security/virtio/core.rst | 49 ++++++++++++++++++++++++++
 MAINTAINERS                            |  1 +
 2 files changed, 50 insertions(+)
 create mode 100644 Documentation/security/virtio/core.rst
  

Comments

Jonathan Corbet Oct. 14, 2022, 2:01 p.m. UTC | #1
Jason Wang <jasowang@redhat.com> writes:

> This patch summarizes the status of hardening and TODO of hardening
> virtio core and drivers.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  Documentation/security/virtio/core.rst | 49 ++++++++++++++++++++++++++
>  MAINTAINERS                            |  1 +
>  2 files changed, 50 insertions(+)
>  create mode 100644 Documentation/security/virtio/core.rst

Do you really need to create a new directory for a single file?

Regardless of where it sits, you'll need to add this file to an
index.rst file so that it becomes part of the docs build.

Thanks,

jon
  
Jason Wang Oct. 17, 2022, 6:09 a.m. UTC | #2
On Fri, Oct 14, 2022 at 10:02 PM Jonathan Corbet <corbet@lwn.net> wrote:
>
> Jason Wang <jasowang@redhat.com> writes:
>
> > This patch summarizes the status of hardening and TODO of hardening
> > virtio core and drivers.
> >
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> >  Documentation/security/virtio/core.rst | 49 ++++++++++++++++++++++++++
> >  MAINTAINERS                            |  1 +
> >  2 files changed, 50 insertions(+)
> >  create mode 100644 Documentation/security/virtio/core.rst
>
> Do you really need to create a new directory for a single file?
>

Not sure, but I think we can start without a dedicated directory.

> Regardless of where it sits, you'll need to add this file to an
> index.rst file so that it becomes part of the docs build.

Yes, I will fix it.

Thanks

>
> Thanks,
>
> jon
>
  
Stefano Garzarella Oct. 18, 2022, 3:39 p.m. UTC | #3
I'm not a native speaker, so the following suggestions can be wrong :-)

On Fri, Oct 14, 2022 at 12:20:37PM +0800, Jason Wang wrote:
>This patch summarizes the status of hardening and TODO of hardening
>virtio core and drivers.
>
>Signed-off-by: Jason Wang <jasowang@redhat.com>
>---
> Documentation/security/virtio/core.rst | 49 ++++++++++++++++++++++++++
> MAINTAINERS                            |  1 +
> 2 files changed, 50 insertions(+)
> create mode 100644 Documentation/security/virtio/core.rst
>
>diff --git a/Documentation/security/virtio/core.rst b/Documentation/security/virtio/core.rst
>new file mode 100644
>index 000000000000..b8baa104d7c8
>--- /dev/null
>+++ b/Documentation/security/virtio/core.rst
>@@ -0,0 +1,49 @@
>+================
>+Virtio hardening
>+================
>+
>+The virtio core and drivers should not trust the devices. This means
>+all kinds of input from the device must be validated before being
>+used. This document summarizes the current status and TODO for this
>+validation/hardening efforts.

s/efforts/effort

>+
>+
>+Status
>+======
>+
>+The virtio core and drivers has done some basic hardening:
>+
>+* Config callback hardening: The core makes sure the config interrupt
>+  callbacks are enabled after the driver is ready and disable before
>+  the driver is removed.
>+
>+* Descriptor ring hardening: The metadata of a descriptor were copied
>+  and stored in a driver private memory that can not be accessed by the
>+  device. The eliminates the device triggerable behaviours through the

s/The/This ? (not sure)
)
>+  descriptor ring.
>+
>+* Device configuration space validation: Some of the virtio drivers
>+  validates the fields of device configuration space before try to use

s/validates/validate

>+  them.
>+
>+
>+TODO
>+====
>+
>+* Input buffer length validation: The virtio core needs to validate
>+  the input buffer length validation before passing them to individual

Remove "validation"?

>+  virtio drivers.
>+
>+* Virtqueue callback hardening: The virtio core (or with the help of
>+  the drivers) should guarantee that the virtqueue callbacks should not
>+  be triggered before the driver is ready or after the driver is
>+  removed.
>+
>+* Transport specific data validation: The virtio transport driver should
>+  validate the virtio transport specific data provided by the device
>+  before trying to use them in the probing.

s/in the probing/during the probe   ?

>+
>+* Device specific validation: Driver should validate the device specific
>+  metadata before being used by a specific subsystem.
>+
>+
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 673b9f9b8d8a..b33cc5c751c1 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -21516,6 +21516,7 @@ S:	Maintained
> F:	Documentation/ABI/testing/sysfs-bus-vdpa
> F:	Documentation/ABI/testing/sysfs-class-vduse
> F:	Documentation/devicetree/bindings/virtio/
>+F:	Documentation/security/virtio/core.rst
> F:	drivers/block/virtio_blk.c
> F:	drivers/crypto/virtio/
> F:	drivers/net/virtio_net.c
>-- 
>2.25.1
>

Thanks,
Stefano
  

Patch

diff --git a/Documentation/security/virtio/core.rst b/Documentation/security/virtio/core.rst
new file mode 100644
index 000000000000..b8baa104d7c8
--- /dev/null
+++ b/Documentation/security/virtio/core.rst
@@ -0,0 +1,49 @@ 
+================
+Virtio hardening
+================
+
+The virtio core and drivers should not trust the devices. This means
+all kinds of input from the device must be validated before being
+used. This document summarizes the current status and TODO for this
+validation/hardening efforts.
+
+
+Status
+======
+
+The virtio core and drivers has done some basic hardening:
+
+* Config callback hardening: The core makes sure the config interrupt
+  callbacks are enabled after the driver is ready and disable before
+  the driver is removed.
+
+* Descriptor ring hardening: The metadata of a descriptor were copied
+  and stored in a driver private memory that can not be accessed by the
+  device. The eliminates the device triggerable behaviours through the
+  descriptor ring.
+
+* Device configuration space validation: Some of the virtio drivers
+  validates the fields of device configuration space before try to use
+  them.
+
+
+TODO
+====
+
+* Input buffer length validation: The virtio core needs to validate
+  the input buffer length validation before passing them to individual
+  virtio drivers.
+
+* Virtqueue callback hardening: The virtio core (or with the help of
+  the drivers) should guarantee that the virtqueue callbacks should not
+  be triggered before the driver is ready or after the driver is
+  removed.
+
+* Transport specific data validation: The virtio transport driver should
+  validate the virtio transport specific data provided by the device
+  before trying to use them in the probing.
+
+* Device specific validation: Driver should validate the device specific
+  metadata before being used by a specific subsystem.
+
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 673b9f9b8d8a..b33cc5c751c1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21516,6 +21516,7 @@  S:	Maintained
 F:	Documentation/ABI/testing/sysfs-bus-vdpa
 F:	Documentation/ABI/testing/sysfs-class-vduse
 F:	Documentation/devicetree/bindings/virtio/
+F:	Documentation/security/virtio/core.rst
 F:	drivers/block/virtio_blk.c
 F:	drivers/crypto/virtio/
 F:	drivers/net/virtio_net.c