Re: [Qemu-devel] AMD SEV's /dev/sev permissions and probing QEMU for capabilities

[Erik Skultety]

On Fri, Jan 18, 2019 at 12:51:50PM +0000, Singh, Brijesh wrote:
>
> On 1/18/19 3:39 AM, Erik Skultety wrote:
> > Hi,
> > this is a summary of a private discussion I've had with guys CC'd on this 
> > email
> > about finding a solution to [1] - basically, the default permissions on
> > /dev/sev (below) make it impossible to query for SEV platform capabilities,
> > since by default we run QEMU as qemu:qemu when probing for capabilities. 
> > It's
> > worth noting is that this is only relevant to probing, since for a proper 
> > QEMU
> > VM we create a mount namespace for the process and chown all the nodes 
> > (needs a
> > SEV fix though).
> >
> > # ll /dev/sev
> > crw-------. 1 root root
> >
> > I suggested either force running QEMU as root for probing (despite the 
> > obvious
> > security implications) or using namespaces for probing too. Dan argued that
> > this would have a significant perf impact and suggested we ask systemd to 
> > add a
> > global udev rule.
> >
> > I proceeded with cloning [1] to systemd and creating an udev rule that I 
> > planned
> > on submitting to systemd upstream - the initial idea was to mimic /dev/kvm 
> > and
> > make it world accessible to which Brijesh from AMD expressed a concern that
> > regular users might deplete the resources (limit on the number of guests
> > allowed by the platform).
>
>
> During private discussion I didn't realized that we are discussing a
> probe issue hence things I have said earlier may not be applicable
> during the probe. The /dev/sev is managed by the CCP (aka PSP) driver.
> The /dev/sev is used for communicating with the SEV FW running inside
> the PSP. The SEV FW offers platform and guest specific services. The
> guest specific services are used during the guest launch, these services
> are available through KVM driver only. Whereas the platform services can
> be invoked at anytime. A typical platform specific services are:
>
> - importing certificates
>
> - exporting certificates
>
> - querying the SEV FW version etc etc
>
> In case of the probe we are not launch SEV guest hence we should not be
> worried about depleting the SEV ASID resources.
>
> IIRC, libvirt uses QEMP query-sev-capabilities to probe the SEV support.
> QEMU executes the below sequence to complete the request:
>
> 1. Exports the platform certificates  (this is when /dev/sev is accessed).
>
> 2. Read the host MSR to determine the C-bit and reduced phys-bit position
>
> I don't see any reason why we can't give world a 'read' permission to
> /dev/sev. Anyone should be able to export the certificates and query

Okay, makes sense to me. The problem I see is the sev_platform_ioctl function
in QEMU which makes an _IOWR request, therefore the file descriptor being
opened in sev_get_capabilities is O_RDWR. Now, I only understand ioctl from
what I've read in the man page, so I don't quite understand the need for IOWR
here - but my honest guess would be that it's because the commands like
SEV_PDH_CERT_EXPORT or SEV_PLATFORM_STATUS need to be copied from userspace to
kernel to instruct kernel which services we want, ergo _IOWR, is that right?
In any case, a fix of some sort needs to land in QEMU first, because no udev
rule would fix the current situation. Afterwards, I expect that having a rule
like this:

KERNEL=="sev", GROUP="kvm", MODE="0644"

and a selinux policy rule adding the kvm_device_t label, we should be fine, do
we agree on that?

> status etc. I think the main issue is reading MSR -- which I believe is
> putting a 'root' requirement. Am I missing something ?
>
>
> > But since the limit is claimed to be around 4, Dan
>
>
> FYI, the limit on EPYC is 15.

Thanks for correction.

Erik