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

["Singh, Brijesh" <brijesh.singh@amd.com>]

On 1/30/19 7:39 AM, Erik Skultety wrote:
>>>> though, we need a #ifdef check for existance of PR_CAP_AMBIENT
>>>>
>>>>> An alternative question I've been playing ever since we exchanged the last few
>>>>> emails is that can't we wait until the ioctls are compared against permissions
>>>>> in kernel so that upstream libvirt (and downstream too for that matter) doesn't
>>>>> have to work around it and stick with that workaround for eternity?
>>>>
>>>> IIUC, the SEV feature has already shipped with distros, so we'd effectively
>>>> be saying that what we already shipped is unusable to libvirt. This doesn't
>>>> feel like a desirable story to me.
>>>
>>> It was, but it never worked, it always has been broken in this way. When we
>>> were merging this upstream, we had a terrible shortage of machines and we had
>>> to share, so the first person to provision the machine had already taken care
>>> of the permissions in order to test so that led to this issue having been
>>> overlooked until now. If it ever worked as expected and then we broke it, then
>>> any fix from our side would make sense but otherwise I believe we should fix
>>> this bottom up.
>>
>> Well technically it would work if libvirt was configured to run as
>> root:root, but yes, that is not a normal or recommended configuration.
>>
>> Personally I have a preference for userspace solutions, as those are
>> pretty straightforward to roll out to people as patches in existing
>> releases. Deploying kernel updates is a higher bar to cross for an
>> existing release.
> 
> So, can you compile the prctl stuff in kernel conditionally? If so, then that's
> a problem because you may end up with a platform where SEV is supported within
> kernel, but you don't have the ambient stuff we have to conditionally compile
> in libvirt, so you end up with broken SEV support anyway, I wanted to argue
> with centos 7, but the ambient set support was backported to 3.10, so the only
> distro where we'd have a problem from userspace POV would be debian 8, but then
> again the kernel there is so old that neither SEV is supported there.
> 


Are you referring to prctl syscall ? If so, I don't think you can
conditionally compile it out. It will be always there. If getting the
libvirt  to run as root:root during the probe is cumbersome and
causing the backward compatibility issues then I guess we can make
/dev/sev 0644. The 0644 will not create any security vulnerability per
say. It may expose us to a DoS attack. e.g a normal user can
open /dev/sev and issue commands to import new certificates and fill the
storage quickly etc. In long run I do want to patch kernel so that a
user without "write" access will not able to issue any command which
will cause the FW to do some flash writes.

In summary, I am against making /dev/sev 0644 if its simplifies the
integrating in libvirt.


> I understand your point, but it also sounds very agile and I don't think that
> compensating with "something that is fast" for "something that is right" is the
> way to go in the long term. Especially since we almost never deprecate stuff
> and we can't break compatibility. Trying to work around every issue coming
> from your dependencies in your project is highly unsustainable.
>