1971 fix virtio issues v1

[samimujawar]

Description

The storage space for virtio-scsi and the virtio-blk request headers being shared with the host was from the stack as the request structure was a local function variable.

This approach is potentially unsafe as the stack data is being shared with the host.

Therefore, allocate memory for the virtio-scsi and virtio-blk request headers from the heap before sharing with the host.

• [ ] Breaking change?
  • No
• [ ] Impacts security?
  • Improves the security as the host cannot accidentally modify the guest stack.
• [ ] Includes tests?
  • No

How This Was Tested

Kvmtool guest firmware built and tested by launching a guest using kvmtool VMM.

Integration Instructions

N/A

[kraxel]

Can you give some more context please? I just don't see how this improves security. The host can access all guest memory anyway. In case of confidential guests the virtio buffers are piped through shared bounce buffers (see OvmfPkg/IoMmuDxe), the stack will not be shared with the host.

[samimujawar]

This is only a minor improvement for a non-CCA guest where should the host software accidentally write to the guest stack area can lead to potential issues.

Apart from this from a design perspective (not related to security), I think sharing stack variables across software components is not good.

[kraxel]

Ok, so it's about CCA guests. It's still not clear to me how moving the allocation from stack to heap improves security. Can the host access memory other than the request struct itself? For example other data on the same page? Should that be the case: Why this isn't a problem for heap allocations?

Some background info on how memory sharing with the host work on CCA would be useful. Pointer to docs is fine.

[samimujawar]

This patch is not applicable for a CCA guest. For a CCA guest bounce buffering as you mentioned would prevent any issue.

For a normal guest VM (non-CCA), a bug in the host could possibly corrupt the guest stack. This sort of an issue can be very hard to debug.

[kraxel]

Yes, memory corruption bugs are indeed hard to debug, because things tend to explode far away from the actual root cause. I fail to see why corrupting the heap instead of the stack is any better though, especially if we are talking about the /host/ corrupting memory. For guest-side corruption heap allocations have the advantage that turning on heap guard can help find the bug. So, yea, a minor improvement.