Re: [PATCH 1/1] MAINTAINERS: introduce cve or security quotient field

[Philippe Mathieu-Daudé]

Hi Prasad,

On 7/14/20 10:36 AM, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> QEMU supports numerous virtualisation and emulation use cases.
> It also offers many features to support guest's function(s).
> 
> All of these use cases and features are not always security relevant.
> Because some maybe used in trusted environments only. Some may still
> be in experimental stage. While other could be very old and not
> used or maintained actively.
> 
> For security bug analysis we generally consider use cases wherein
> QEMU is used in conjunction with the KVM hypervisor, which enables
> guest to use hardware processor's virtualisation features.
> 
> The CVE (or Security or Trust) Quotient field tries to capture this
> sensitivity pertaining to a feature or section of the code.
> 
> It indicates whether a potential issue should be treated as a security
> one OR it could be fixed as a regular non-security bug.
> 
> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  MAINTAINERS | 324 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 324 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fe8139f367..badf1dab6e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -33,6 +33,14 @@ Descriptions of section entries:
>          Obsolete:    Old code. Something tagged obsolete generally means
>                       it has been replaced by a better system and you
>                       should be using that.
> +     C: CVE/Security/Trust Quotient
> +        H:High - Feature (or code) is meant to be safe and used by untrusted
> +                 guests. So any potential security issue must be processed 
> with
> +                 due care and be considered as a CVE issue.
> +        L:Low  - Feature (or code) is not meant to be safe OR is experimental
> +                 OR is used in trusted environments only OR is not well
> +                 maintained. So any potential security issue can be processed
> +                 and fixed as regular non-security bug. No need for a CVE.

I'm not sure we need this separation of good/bad citizen.

We already have the 'S' flag:

        S: Status, one of the following:
           Supported:   Someone is actually paid to look after this.
           Maintained:  Someone actually looks after it.
           Odd Fixes:   It has a maintainer but they don't have time to
                        do much other than throw the odd patch in. See
                        below.
           Orphan:      No current maintainer [but maybe you could take
                        the role as you write your new code].
           Obsolete:    Old code. Something tagged obsolete generally
                        means it has been replaced by a better system
                        and you should be using that.

I think the 'Supported' status already describes a feature that
important enough to a company to have an employee looking at it.

If a section is not 'Supported', it is unlikely the maintainer
have time to deal with security issues.

I disagree with the High/Low tag, but I still reviewed the rest.

>       F: Files and directories with wildcard patterns.
>          A trailing slash includes all files and subdirectory files.
>          F:   drivers/net/    all files in and below drivers/net
> @@ -87,6 +95,7 @@ S390 general architecture support
>  M: Cornelia Huck <cohuck@redhat.com>
>  M: Thomas Huth <thuth@redhat.com>
>  S: Supported
> +C: High
>  F: default-configs/s390x-softmmu.mak
>  F: gdb-xml/s390*.xml
>  F: hw/char/sclp*.[hc]

> @@ -149,6 +161,7 @@ ARM TCG CPUs
>  M: Peter Maydell <peter.maydell@linaro.org>
>  L: qemu-arm@nongnu.org
>  S: Maintained
> +C: Low
>  F: target/arm/

There is KVM code there.

>  F: tests/tcg/arm/
>  F: tests/tcg/aarch64/
> @@ -164,6 +177,7 @@ ARM SMMU
>  M: Eric Auger <eric.auger@redhat.com>
>  L: qemu-arm@nongnu.org
>  S: Maintained
> +C: High
>  F: hw/arm/smmu*
>  F: include/hw/arm/smmu*
>  
...

> @@ -270,6 +294,7 @@ PowerPC TCG CPUs
>  M: David Gibson <david@gibson.dropbear.id.au>
>  L: qemu-ppc@nongnu.org
>  S: Maintained
> +C: High

You might want to split this section in 2 to keep various
areas in Low.

>  F: target/ppc/
>  F: hw/ppc/
>  F: include/hw/ppc/
...

> @@ -440,6 +482,7 @@ M: Cameron Esfahani <dirty@apple.com>
>  M: Roman Bolshakov <r.bolshakov@yadro.com>
>  W: https://wiki.qemu.org/Features/HVF
>  S: Maintained
> +C: Low
>  F: accel/stubs/hvf-stub.c
>  F: target/i386/hvf/
>  F: include/sysemu/hvf.h
> @@ -447,6 +490,7 @@ F: include/sysemu/hvf.h
>  WHPX CPUs
>  M: Sunil Muthuswamy <sunilmut@microsoft.com>
>  S: Supported
> +C: Low

I think this is High.

>  F: target/i386/whpx-all.c
>  F: target/i386/whp-dispatch.h
>  F: accel/stubs/whpx-stub.c
> @@ -460,6 +504,7 @@ M: Anthony Perard <anthony.perard@citrix.com>
>  M: Paul Durrant <paul@xen.org>
>  L: xen-devel@lists.xenproject.org
>  S: Supported
> +C: High
>  F: */xen*
>  F: accel/xen/*
>  F: hw/9pfs/xen-9p*
> @@ -486,6 +531,7 @@ M: Colin Xu <colin.xu@intel.com>
>  L: haxm-team@intel.com
>  W: https://github.com/intel/haxm/issues
>  S: Maintained
> +C: Low

Ditto.

>  F: accel/stubs/hax-stub.c
>  F: include/sysemu/hax.h
>  F: target/i386/hax-*
> @@ -497,12 +543,14 @@ M: Michael S. Tsirkin <mst@redhat.com>
>  M: Cornelia Huck <cohuck@redhat.com>
>  M: Paolo Bonzini <pbonzini@redhat.com>
>  S: Maintained
> +C: High
>  F: linux-headers/
>  F: scripts/update-linux-headers.sh
...
> @@ -1631,11 +1782,13 @@ Character devices
>  M: Marc-André Lureau <marcandre.lureau@redhat.com>
>  R: Paolo Bonzini <pbonzini@redhat.com>
>  S: Odd Fixes
> +C: High
>  F: hw/char/
>  
>  Network devices
>  M: Jason Wang <jasowang@redhat.com>
>  S: Odd Fixes
> +C: High
>  F: hw/net/
>  F: include/hw/net/
>  F: tests/qtest/virtio-net-test.c

These two don't make sense to me. You can not be low class citizen
only maintained for 'Odd Fixes' and aim for security. Choose one.

>  SD (Secure Card)
>  M: Philippe Mathieu-Daudé <f4bug@amsat.org>
>  S: Odd Fixes
> +C: Low
>  F: include/hw/sd/sd*
>  F: hw/sd/core.c
>  F: hw/sd/sd*
> @@ -1684,6 +1842,7 @@ F: tests/qtest/sd*
>  USB
>  M: Gerd Hoffmann <kraxel@redhat.com>
>  S: Maintained
> +C: High
>  F: hw/usb/*

Similarly to PPC, you might want to split this one to reduce
coverage.

>  F: tests/qtest/usb-*-test.c
>  F: docs/usb2.txt
> @@ -1696,11 +1855,13 @@ USB (serial adapter)
>  M: Gerd Hoffmann <kraxel@redhat.com>
>  M: Samuel Thibault <samuel.thibault@ens-lyon.org>
>  S: Maintained
> +C: High
>  F: hw/usb/dev-serial.c
...

>  tulip
>  M: Sven Schnelle <svens@stackframe.org>
>  S: Maintained
> +C: High

Low.

>  F: hw/net/tulip.c
>  F: hw/net/tulip.h
>  
>  Generic Loader
>  M: Alistair Francis <alistair@alistair23.me>
>  S: Maintained
> +C: High
>  F: hw/core/generic-loader.c
>  F: include/hw/core/generic-loader.h
>  F: docs/generic-loader.txt

I'm not sure about this one.

> @@ -1921,12 +2108,14 @@ F: docs/generic-loader.txt
>  Intel Hexadecimal Object File Loader
>  M: Su Hang <suhang16@mails.ucas.ac.cn>
>  S: Maintained
> +C: Low
>  F: tests/qtest/hexloader-test.c
>  F: tests/data/hex-loader/test.hex
...

>  EDID Generator
>  M: Gerd Hoffmann <kraxel@redhat.com>
>  S: Maintained
> +C: Low
>  F: hw/display/edid*
>  F: include/hw/display/edid.h
>  F: qemu-edid.c

I'm not sure, but maybe.

> @@ -2012,6 +2211,7 @@ PIIX4 South Bridge (i82371AB)
>  M: Hervé Poussineau <hpoussin@reactos.org>
>  M: Philippe Mathieu-Daudé <f4bug@amsat.org>
>  S: Maintained
> +C: High

No, this one is low (which is why it has is own section,
to not bother MST).

>  F: hw/isa/piix4.c
>  F: include/hw/southbridge/piix.h
...

>  Device Tree
>  M: Alistair Francis <alistair.francis@wdc.com>
>  R: David Gibson <david@gibson.dropbear.id.au>
>  S: Maintained
> +C: Low
>  F: device_tree.c
>  F: include/sysemu/device_tree.h

This one is consumed by the Virt machine, maybe High?

>  
>  Dump
>  S: Supported
> +C: Low
>  M: Marc-André Lureau <marcandre.lureau@redhat.com>
>  F: dump/
>  F: hw/misc/vmcoreinfo.c
...

>  QObject
>  M: Markus Armbruster <armbru@redhat.com>
>  S: Supported
> +C: Low
>  F: qobject/
>  F: include/qapi/qmp/
>  X: include/qapi/qmp/dispatch.h

Low? Odd.

> @@ -2385,6 +2620,7 @@ T: git https://repo.or.cz/qemu/armbru.git qapi-next
>  QEMU Guest Agent
>  M: Michael Roth <mdroth@linux.vnet.ibm.com>
>  S: Maintained
> +C: Low

Odd too.

>  F: qga/
>  F: docs/interop/qemu-ga.rst
>  F: scripts/qemu-guest-agent/
> @@ -2397,6 +2633,7 @@ M: Paolo Bonzini <pbonzini@redhat.com>
>  R: Daniel P. Berrange <berrange@redhat.com>
>  R: Eduardo Habkost <ehabkost@redhat.com>
>  S: Supported
> +C: High
>  F: docs/qdev-device-use.txt
>  F: hw/core/qdev*
>  F: hw/core/bus.c
...

>  Register API
>  M: Alistair Francis <alistair@alistair23.me>
>  S: Maintained
> +C: High

No, Low.

>  F: hw/core/register.c
>  F: include/hw/register.h
>  F: include/hw/registerfields.h
> @@ -2456,6 +2697,7 @@ F: include/hw/registerfields.h
...

>  Tracing
>  M: Stefan Hajnoczi <stefanha@redhat.com>
>  S: Maintained
> +C: Low

Some backends are High.

>  F: trace/
>  F: trace-events
>  F: docs/qemu-option-trace.rst.inc
> @@ -2488,6 +2733,7 @@ T: git https://github.com/stefanha/qemu.git tracing
>  TPM
>  M: Stefan Berger <stefanb@linux.ibm.com>
>  S: Maintained
> +C: Low

High!!!

>  F: tpm.c
>  F: stubs/tpm.c
>  F: hw/tpm/*
> @@ -2500,12 +2746,14 @@ T: git https://github.com/stefanberger/qemu-tpm.git 
> tpm-next
...

> @@ -2601,6 +2859,7 @@ L: qemu-block@nongnu.org
>  UUID
>  M: Fam Zheng <fam@euphon.net>
>  S: Supported
> +C: Low

High?

>  F: util/uuid.c
>  F: include/qemu/uuid.h
>  F: tests/test-uuid.c
> @@ -2608,6 +2867,7 @@ F: tests/test-uuid.c
...

>  Null Block Driver
>  M: Fam Zheng <fam@euphon.net>
>  L: qemu-block@nongnu.org
>  S: Supported
> +C: Low

High?

>  F: block/null.c
>  
>  NVMe Block Driver
>  M: Fam Zheng <fam@euphon.net>
>  L: qemu-block@nongnu.org
>  S: Supported
> +C: Low

Certainly High.

>  F: block/nvme*
>  
>  Bootdevice
>  M: Gonglei <arei.gonglei@huawei.com>
>  S: Maintained
> +C: Low
>  F: bootdevice.c
...

>  Replication
>  M: Wen Congyang <wencongyang2@huawei.com>
>  M: Xie Changlong <xiechanglong.d@gmail.com>
>  S: Supported
> +C: Low

High?

>  F: replication*
>  F: block/replication.c
>  F: tests/test-replication.c
> @@ -2997,6 +3309,7 @@ PVRDMA
>  M: Yuval Shaia <yuval.shaia.ml@gmail.com>
>  M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>  S: Maintained
> +C: High
>  F: hw/rdma/*
>  F: hw/rdma/vmw/*
>  F: docs/pvrdma.txt
...