libvmmapi: Map the guest physical address space with PROT_CAP #2274
Conversation
| @@ -415,7 +415,7 @@ setup_memory_segment(struct vmctx *ctx, vm_paddr_t gpa, size_t len, char *base) | |||
| flags |= MAP_NOCORE; | |||
|
|
|||
| /* mmap into the process address space on the host */ | |||
| ptr = mmap(base + gpa, len, PROT_RW, flags, ctx->fd, gpa); | |||
| ptr = mmap(base + gpa, len, PROT_RW | PROT_CAP, flags, ctx->fd, gpa); | |||
There was a problem hiding this comment.
I probably broke this in d2f37a3. With this change we generally don't add capability permissions to non-anonymous mappings.
There was a problem hiding this comment.
To be clear, this is the intended behaviour?
There was a problem hiding this comment.
I think it probably is. I think we probably believe that MAP_SHARED file mappings that need capability permissions will be sufficiently rare that requiring an explicit PROT_CAP is not a deal breaker in terms of compatibility.
There was a problem hiding this comment.
MAP_SHARED mappings with should have to request PROT_CAP to ensure the developer means it as MAP_SHARED mappings in general present a way for capabilities to leak between address spaces where they may cause aliasing and temporal safety violations.
That does make me wonder if it's actually safe to sweep these mappings during revocation. If they only contain kernel pointers then it should be fine, but if they contain user pointers we might blow things up quite excitingly.
There was a problem hiding this comment.
Hmm, I hadn't really considered that. This is a mapping of the guest physical address space, so I see no reason that we wouldn't encounter user pointers. We can't safely sweep this mapping, nor can we leave it alone with PROT_CAP set without breaking temporal safety within bhyve itself.
Since PROT_CAP is needed only for the gdb stub, at least so far, perhaps the solution is to ask the kernel to fetch the capability tag for us?
There was a problem hiding this comment.
I can see an argument for requiring a system call to read the capability. Other users of this mapping (which basically exist to implement DMA) should be tag-oblivious and in particular, they should always clear tags on writes. Also, revocation would definitely blow up here and do bad things to the guest's address space.
There was a problem hiding this comment.
I'll work on that. I was thinking of adding a vmmdev ioctl to handle this; do you think it makes sense to have a more generic syscall for this purpose.
There was a problem hiding this comment.
It also avoids confused deputy attacks on the userspace part of the hypervisor by being able to inject capabilities via this mapping.
This lets the gdb stub see capability tag bits; otherwise, they're cleared on load and the debugger sees all capabilities as invalid. Note that this enables the writing of capabilities to the GPA space as well.
markjdb
force-pushed
the
dev-libvmmapi-prot-cap
branch
from
e84bc1d to
ebb6b74
Compare
No description provided.