Message-ID: <Z3yK0Ee3eFLocJDW@liuwe-devbox-debian-v2>
Date: Tue, 7 Jan 2025 02:00:48 +0000
From: Wei Liu <wei.liu@...nel.org>
To: Michael Kelley <mhklinux@...look.com>
Cc: Sonia Sharma <sosha@...ux.microsoft.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"sosha@...rosoft.com" <sosha@...rosoft.com>,
	"decui@...rosoft.com" <decui@...rosoft.com>,
	"ssengar@...ux.microsoft.com" <ssengar@...ux.microsoft.com>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	Wei Liu <wei.liu@...nel.org>
Subject: Re: [PATCH] Drivers: hv: Allow single instance of hv_util devices

On Sun, Dec 29, 2024 at 06:02:34PM +0000, Michael Kelley wrote:
> From: Sonia Sharma <sosha@...ux.microsoft.com> Sent: Friday, December 20, 2024 3:56 PM
> > 
> 
> Please include the "linux-hyperv@...r.kernel.org" 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.
> >

Why is this needed? What's the problem that this patch is trying to
solve?

> 
> 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.

#2 is preferred. It is frowned upon to make assumptions that only one
instance of a device will be present.

It perhaps takes more work to check and enforce the invariant (as this
patch demonstrates) than to just let the device framework handle
multiple instances.

Thanks,
Wei.

Powered by blists - more mailing lists