RE: [PATCH] Drivers: hv: Allow single instance of hv_util devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Sonia Sharma <sosha@xxxxxxxxxxxxxxxxxxx> Sent: Friday, December 20, 2024 3:56 PM
> 

Please include the "linux-hyperv@xxxxxxxxxxxxxxx" mailing list
when submitting patches related to Hyper-V.

> Harden hv_util type device drivers to allow single
> instance of the device be configured at given time.
>

I think the reason for this patch needs more explanation. For several
VMBus devices, a well-behaved Hyper-V is expected to offer only one
instance of the device in a given VM. Linux guests originally assumed
that the Hyper-V host is well-behaved, so the device drivers for many
of these devices were written assuming only a single instance. But
with the introduction of Confidential Computing (CoCo) VMs, Hyper-V
is no longer assumed to be well-behaved. If a compromised & malicious
Hyper-V were to offer multiple instances of such a device, the device
driver assumption about a single instance would be false, and
memory corruption could occur, which has the potential to lead to
compromise of the CoCo VM. The intent is to prevent such a scenario.

Note that this problem extends beyond just "util" devices. Hyper-V
is expected to offer only a single instance of keyboard, mouse, frame
buffer, and balloon devices as well. So this patch should be extended
to include them as well (and your new function names containing
"hv_util" should be adjusted). Interestingly, the Hyper-V keyboard driver
does not assume a single instance, so it should be safe regardless. But
the mouse, frame buffer, and balloon drivers are not safe.

With this understanding, there are two ways to approach the problem:

1) Enforce the expectation that a well-behaved Hyper-V only offers a
single instance of these VMBus devices. That's the approach that this
patch takes.

2) Update the device drivers to remove the assumption of a single
instance. With this approach, if a compromised & malicious Hyper-V
were to offer multiple instances, the extra devices might be bogus, 
but memory corruption would not occur and the integrity of the
CoCo VM should not be compromised. As mentioned above, such
is already the case with the keyboard driver.

I've thought about the tradeoffs for the two approaches, and don't
really have a strong opinion either way. In some sense, #2 is the
more correct approach as ideally device drivers shouldn't make
single instance assumptions. But #1 is an easier fix, and perhaps
more robust. Other reviewers might have other reasons to prefer
one over the other, and have a stronger viewpoint on the tradeoffs.
I would be interested in any such comments. But I'm OK with
approach #1 unless someone points out a good reason to
prefer #2.

>
> New function vmbus_is_valid_hvutil_offer() is added.
> It checks if the new offer is for hv_util device type,
> then read the refcount for that device and accept or
> reject the offer accordingly.
> 
> Signed-off-by: Sonia Sharma <sonia.sharma@xxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/hv/channel_mgmt.c | 64 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 3c6011a48dab..1a135cfad81f 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -20,6 +20,7 @@
>  #include <linux/delay.h>
>  #include <linux/cpu.h>
>  #include <linux/hyperv.h>
> +#include <linux/refcount.h>
>  #include <asm/mshyperv.h>
>  #include <linux/sched/isolation.h>
> 
> @@ -156,6 +157,8 @@ const struct vmbus_device vmbus_devs[] = {
>  };
>  EXPORT_SYMBOL_GPL(vmbus_devs);
> 
> +static refcount_t singleton_vmbus_devs[HV_UNKNOWN + 1];
> +


[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux