Re: [PATCH v1 0/2] Add support to disable VirtIo net at runtime

["Laszlo Ersek" <lersek@redhat.com>]

On 08/04/22 04:52, Yuan Yu wrote:
> Currently networking can only be enabled/disabled at compile time. This
> patch series will add support to disable VirtIo net at runtime even if
> the functionality is built into binary at compile time.
> 
> This will enable VMM to reduce attack surface without recompilation.
> 
> The changes can be seen at:
> https://github.com/yyu/edk2/tree/network_cfg_lib_v1
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
> 
> Yuan Yu (2):
>   OvmfPkg: Introduce NetworkCfgLib
>   OvmfPkg: Use PcdNetworkSupport to enable/disable VirtIo net
> 
>  OvmfPkg/OvmfPkg.dec                             |  3 ++
>  OvmfPkg/OvmfPkgX64.dsc                          |  7 ++++-
>  OvmfPkg/Library/NetworkCfgLib/NetworkCfgLib.inf | 29 ++++++++++++++++++
>  OvmfPkg/VirtioNetDxe/VirtioNet.inf              |  3 ++
>  OvmfPkg/Library/NetworkCfgLib/NetworkCfgLib.c   | 32 ++++++++++++++++++++
>  OvmfPkg/VirtioNetDxe/EntryPoint.c               | 10 ++++++
>  6 files changed, 83 insertions(+), 1 deletion(-)
>  create mode 100644 OvmfPkg/Library/NetworkCfgLib/NetworkCfgLib.inf
>  create mode 100644 OvmfPkg/Library/NetworkCfgLib/NetworkCfgLib.c
> 

Well I've not been reviewing upstream edk2 patches for a while, but the
virtio-net driver is still very close to my heart, so this patch kind of
hits a nerve.

I think I disagree with the idea and the implementation both.

Minimally, the idea needs a much better elaboration -- what is the
threat model? Do you want to protect the host from the guest, or the
guest from the host? Or something else? How does controlling a single
SNP driver via fw_cfg (which is also dictated by the host) help?

Regarding the implementation: there is much more to networking in edk2
than VirtioNetDxe. UEFI driver binaries (SNP drivers) built from iPXE
can be passed in via the NICs' option ROMs. SNP drivers can be loaded
from the UEFI system partition (for example, Intel's binary-only driver
for QEMU's e1000* cards).

If you can control this fw_cfg switch from the VMM side, you can also
control the VMM enough to simply *not give* a virtio-net device to the
guest. Then the driver (it being a UEFI driver following the UEFI driver
model) will simply not have anything to bind.

Sorry I find this approach very wrong. If you really need it for your
particular VMM, I kind of suggest not upstreaming this patch. I see it
as a step backwards for the upstream project.

Anyway: I emphasize I'm no longer a reviewer for OvmfPkg, and it's quite
exceptional that I'm reacting at all to being CC'd on this.

Laszlo