Re: [virtio-dev] [PATCH RFC 3/3] rng: leak detection support
On 27/9/23 23:47, Michael S. Tsirkin wrote: On Wed, Sep 27, 2023 at 12:43:20PM +0200, Babis Chalios wrote: On 22/9/23 18:01, Michael S. Tsirkin wrote: On Fri, Sep 22, 2023 at 05:40:50PM +0200, Babis Chalios wrote: On 22/9/23 17:06, Michael S. Tsirkin wrote: On Tue, Sep 19, 2023 at 12:11:37PM +0200, Babis Chalios wrote: On 19/9/23 12:01, Michael S. Tsirkin wrote: On Tue, Sep 19, 2023 at 09:32:08AM +0200, Babis Chalios wrote: Resending to fix e-mail formatting issues (sorry for the spam) On 18/9/23 18:30, Babis Chalios wrote: Yes, that's what the driver does now in the RFC patch. However, this just decreases the race window, it doesn't eliminate it. If a third leak event happens it might not find any buffers to use: 1. available buffers to queue 1-X 2. available buffers to queue X 3. poll queue X 4. used buffers in queue X <- leak event 1 will use buffers in X 5. avail buffers in queue X 6. poll queue 1-X<- leak event 2 will use buffers in 1-X 7. used buffers in queue 1-X 8. avail buffers in queue 1-X <- leak event 3 (it needs buffers in X, race with step 5) 9. goto 3 I don't get it. we added buffers in step 5. What if the leak event 3 arrives before step 5 had time to actually add the buffers in X and make them visible to the device? Then it will see a single event in 1-X instead of two events. A leak is a leak though, I don't see does it matter how many triggered. So the scenario I have in mind is the following: (Epoch here is terminology that I used in the Linux RFC. It is a value maintained by random.c that changes every time a leak event happens). 1. add buffers to 1-X 2. add buffers to X 3. poll queue X 4. vcpu 0: get getrandom() entropy and cache epoch value 5. Device: First snapshot, uses buffers in X 6. vcpu 1: sees used buffers 7. Device: Second snapshot, uses buffers in 1-X 8. vcpu 0: getrandom() observes new epoch value & caches it 9. Device: Third snapshot, no buffers in either queue, (vcpu 1 from step 6 has not yet finished adding new buffers). 10. vcpu 1 adds new buffer in X 11. vcpu 0: getrandom() will not see new epoch and gets stale entropy. In this succession of events, when the third snapshot will happen, the device won't find any buffers in either queue, so it won't increase the RNG epoch value. So, any entropy gathered after step 8 will be the same across all snapshots. Am I missing something? Cheers, Babis Yes but notice how this is followed by: 12. vcpu 1: sees used buffers in 1-X Driver can notify getrandom I guess? It could, but then we have the exact race condition that VMGENID had, userspace has already consumed stale entropy and there's nothing we can do about that. Although this is indeed a corner case, it feels like it beats the purpose of having the hardware update directly userspace (via copy on leak). How do you feel about the proposal a couple of emails back? It looks to me that it avoids completely the race condition. Cheers, Babis It does. The problem of course is that this means that e.g. taking a snapshot of a guest that is stuck won't work well. That is true, but does it matter? The intention of the proposal is that if it is not safe to take snapshots (i.e. no buffers in the queue) don't take snapshots. I have been thinking of adding MAP/UNMAP descriptors for a while now. Thus it will be possible to modify userspace memory without consuming buffers. Would something like this solve the problem? I am not familiar with MAP/UNMAP descriptors. Is there a link where I can read about them? Cheers, Babis Heh no I just came up with the name. Will write up in a couple of days, but the idea is that driver does get_user_pages, adds buffer to queue, and device will remember the address and change that memory on a snapshot. If there are buffers in the queue it will also use these to tell driver, but if there are no buffers then it won't. That sounds like a nice mechanism. However in our case the page holding the counter that gets increased by the hardware is a kernel page. The reason for that is that things other than us (virtio-rng) might want to notify for leak events. For example, I think that Jason intended to use this mechanism to periodically notify user-space PRNGs that they need to reseed. Cheers, Babis Now I'm lost. when you write, e.g.: 4. vcpu 0: get getrandom() entropy and cache epoch value how does vcpu access the epoch? The kernel provides a user space API to map a pointer to the epoch value. User space then caches its value and checks it every time it needs to make sure that no entropy leak has happened before using cached kernel entropy. virtio-rng driver adds a copy on leak command to the queue for increasing this value (that's what we are speaking about in this thread). But other systems might want to report "leaks", such as random.c itself. Cheers, Babis - To unsubscribe, e-mail:
Re: [virtio-dev] [PATCH v2] virtio-tee: Reserve device ID 46 for TEE device
On Wed, 27 Sept 2023 at 21:39, Arnd Bergmann wrote: > > On Wed, 27 Sept 2023 at 16:09, Sumit Garg wrote: > > > > It looks like I accidentally dropped the entire Cc list in my earlier reply, > adding them all back. No worries. > > > On Wed, 27 Sept 2023 at 01:44, Arnd Bergmann wrote: > > > > > > On Tue, 26 Sept 2023 at 08:44, Sumit Garg wrote: > > > > On Tue, 26 Sept 2023 at 08:16, Jens Wiklander > > > > wrote: > > > > > On Tue, Sep 26, 2023 at 8:00 AM Sumit Garg > > > > > wrote: > > > > > > > > > > > > How about shared memory support? We would like to register guest > > > > > > pages with the trusted OS. > > > > > > > > > > Coincidently Arnd and I (among others) discussed this in person last > > > > > week and the conclusion was that only temporary shared memory is > > > > > possible with virtio. So the shared memory has to be set up and torn > > > > > down by the host during each operation, typically open-session or > > > > > invoke-func. > > > > > > > > Agree as I was part of those discussions. But I would like to > > > > understand the reasoning behind it. Is there any restriction by VIRTIO > > > > specification that we can't register guest page PAs to a device (TEE > > > > in our case) to allow for zero copy transfers? > > > > > > > > Alex mentioned some references to virtio GPU device. I suppose I need > > > > to dive into its implementation to see if there are any similarities > > > > to our use-case. > > > > > > > > > That might not be optimal if trying to maximize > > > > > performance, but it is portable. > > > > > > > > IMO, the ABI should be flexible enough to support a TEE with optimum > > > > performance. > > > > > > As we discussed last week, I can see two possible ways to implement > > > a TEE device within the constraints of the virtio specification: > > > > > > a) Allocate a shared memory area in the device (host) and export it > > >to the driver (guest) via a virtio shared memory area. This shared > > >memory can be shared with userspace using mmap() if necessary. > > >A tee command in this case would be sent using a normal virtqueue > > >to the device with a pair of one transmit and one receive buffer. > > >Any arguments that refer to memory blocks in this case are > > >offsets into the shared memory area. Using this preallocated buffer > > >is similar to earlier TEE implementations but has some restrictions. > > >The command in this case has to be copied in and out by the > > >hypervisor implementation. > > > > Yeah that was the initial approach that we used for OP-TEE but it was > > limited by the fixed size of the shared memory area. A guest client > > application may want to share a larger data buffer with TA which can't > > be supported via this approach. > > I don't know if there is a limit on the size of the virtio shared > memory area other than the PCI MMIO space size, could > this just be made (much) larger? Actually it's a double edged sword. You wouldn't like to block too much host memory which remains unused and on the other hand not too less that can't serve guest application needs. That's the reason we should provide an option to dynamically share guest memory with a TEE. > > > > b) Send all data through the virtqueue itself, pointing into normal > > >guest memory. The first buffer sent to the device is the request, > > >while the receive buffer is the result. Instead of pointers to > > >shared memory, this means that all data transfers would be > > >done in additional buffers on the same virtio transaction, and > > >the host would have to register the guest memory dynamically > > >as part of the command before forwarding them to a TEE that > > >relies on registering shared memory, and unmap it afterwards > > >since the guest might reuse the buffers for other data later > > >that it does not want to share with the TEE > > > > > > > The TEE communication protocol has to support INOUT shared memory > > buffers. So it will be quite tricky to support it via TX only and RX > > only buffers (many more buffer copies). > > I don't remember if you can just list the same address in > virtio for both directions, but that could solve this problem. > Okay in that case I think following approach can work: - Issue VIRTIO_TEE_CMD_REGISTER_MEM to register an additional buffer passed through virtqueue. - Instruct TEE to perform operations on it via VIRTIO_TEE_CMD_INVOKE_FUNC. - Issue VIRTIO_TEE_CMD_UNREGISTER_MEM to unregister that additional buffer passed through virtqueue. I suppose we can pass either guest user-space or kernel reference in that virtqueue. Does this approach make sense to you? -Sumit > > > Registering guest memory to the TEE permanently would be > > > a layering violation since that makes invalid assumptions about > > > the type of virtio transport that do not make sense to a virtio > > > driver. > > > > AFAICS, the VIRTIO is just a transport to relay information among > > guest
Re: [virtio-dev] [PATCH] html: add missing enumitem package
On Wed, Sep 27 2023, "Michael S. Tsirkin" wrote:
> makediffhtml.sh currently fails with:
>
> ! Missing number, treated as zero.
>
>\c@*
> l.25850 \begin{enumerate}[label=\alph*
> .]
> ?
> ! Emergency stop.
>
>\c@*
> l.25850 \begin{enumerate}[label=\alph*
> .]
>
> Some web searches turned up suggestions to use enumitem and in fact,
> virtio.tex already does this - but virtio-html.tex doesn't.
>
> Adding \usepackage{enumitem} in virtio-html.tex too fixes the issue.
>
> Signed-off-by: Michael S. Tsirkin
> ---
> virtio-html.tex | 1 +
> 1 file changed, 1 insertion(+)
>
Oops, should read my mail before pushing. I'll use this one instead.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v3] virtio-tee: Reserve device ID 46 for TEE device
In a virtual environment, an application running in guest VM may want
to delegate security sensitive tasks to a Trusted Application (TA)
running within a Trusted Execution Environment (TEE). A TEE is a trusted
OS running in some secure environment, for example, TrustZone on ARM
CPUs, or a separate secure co-processor etc.
A virtual TEE device emulates a TEE within a guest VM. Such a virtual
TEE device supports multiple operations such as:
VIRTIO_TEE_CMD_OPEN_DEVICE – Open a communication channel with virtio
TEE device.
VIRTIO_TEE_CMD_CLOSE_DEVICE – Close communication channel with virtio
TEE device.
VIRTIO_TEE_CMD_GET_VERSION – Get version of virtio TEE.
VIRTIO_TEE_CMD_OPEN_SESSION – Open a session to communicate with
trusted application running in TEE.
VIRTIO_TEE_CMD_CLOSE_SESSION – Close a session to end communication
with trusted application running in TEE.
VIRTIO_TEE_CMD_INVOKE_FUNC – Invoke a command or function in trusted
application running in TEE.
VIRTIO_TEE_CMD_CANCEL_REQ – Cancel an ongoing command within TEE.
VIRTIO_TEE_CMD_REGISTER_MEM - Register shared memory with TEE.
VIRTIO_TEE_CMD_UNREGISTER_MEM - Unregister shared memory from TEE.
We would like to reserve device ID 46 for Virtio-TEE device.
Signed-off-by: Jeshwanth Kumar
Reviewed-by: Rijo Thomas
Reviewed-by: Parav Pandit
Acked-by: Sumit Garg
---
content.tex | 2 ++
1 file changed, 2 insertions(+)
diff --git a/content.tex b/content.tex
index 0a62dce..644aa4a 100644
--- a/content.tex
+++ b/content.tex
@@ -739,6 +739,8 @@ \chapter{Device Types}\label{sec:Device Types}
\hline
45 & SPI master \\
\hline
+46 & TEE device \\
+\hline
\end{tabular}
Some of the devices above are unspecified by this document,
--
2.25.1
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
