virtio-rng based entropy leak reporting

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.

Add virtqueues to support reporting entropy leaks (similar to virtio based vmgenid).

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  virtio-rng.tex | 61 +++++++++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 60 insertions(+), 1 deletion(-)

virtio-rng.tex +60 -2

index 1ec7164..4760dfa 100644
@@ -9,10 +9,14 @@ \subsection{Device ID}\label{sec:Device Types / Entropy Device / Device ID}
  \subsection{Virtqueues}\label{sec:Device Types / Entropy Device / Virtqueues}
  \begin{description}
  \item[0] requestq
+\item[1] leakq1 (only if VIRTIO_RNG_F_LEAK is offered)
+\item[2] leakq2 (only if VIRTIO_RNG_F_LEAK is offered)
  \end{description}
 
  \subsection{Feature bits}\label{sec:Device Types / Entropy Device / Feature bits}
-  None currently defined
+\begin{description}
+\item[VIRTIO_RNG_F_LEAK (0)] Device can report and handle information leaks.
+\end{description}
 
  \subsection{Device configuration layout}\label{sec:Device Types / Entropy Device / Device configuration layout}
    None currently defined.

@@ -21,6 +25,7 @@ \subsection{Device Initialization}\label{sec:Device Types / Entropy Device / Dev
 
  \begin{enumerate}
  \item The \field{requestq} virtqueue is initialized
+\item If VIRTIO_RNG_F_LEAK has been negotiated, \field{leakq1} and \field{leakq2} are initialized
  \end{enumerate}
 
  \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device Operation}

@@ -41,3 +46,57 @@ \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O
  The device MUST place one or more random bytes into the buffer
  made available to it through \field{requestq}, but it
  MAY use less than the entire buffer length.
+
+\subsubsection{Reporting Information Leaks}{Device Types / Entropy Device / Device Operation / Reporting Information Leaks}
+
+The device might, after the fact, detect that some of the entropy
+it supplied to the driver has after the fact degraded in quality
+or leaked to the outside world.  One example is when the device
+is part of the virtual machine undergoing a restore from snapshot
+operation. Another example is when the information leaks from the
+host system through a side-channel.
+
+The driver would typically react by causing regeneration of any
+information that might have leaked and that has to be secret or
+unique.  It is understood that when an information leak has been
+detected it is likely not limited to the entropy received through
+the specific device. In particular, this is the case for
+snapshoting It is thus suggested that the system fully
+regenerate any unique/secret information in this scenario.
+
+If VIRTIO_RNG_F_LEAK has been negotiated the device can report
+such leaks to the driver through a set of dedicated leak
+queues: \field{leakq1} and \field{leakq2}.
+
+Buffers added to the leak queues can have one of two forms:
+\begin{enumerate}
+\item A write-only buffer. It will be completely filled by random data by the device.
+\item A buffer consisting of read-only section followed by a
+write-only section, both of identical size. The
+device will copy data from the read-only section to the write-only
+section.
+\end{enumerate}
+
+The steps for operating the virtqueue are:
+
+\begin{enumerate}
+\item At each time, only one of \field{leakq1}, \field{leakq2} is active
+      (has buffers added/used).
+\item After initialization, \field{leakq1} is active.
+\item Driver adds multiple buffers to the active leak queue.
+\item The buffers are not used until an information leak is
+      detected, as long as that is the case driver can
+      add more buffers to the active queue.
+\item Upon detecting an information leak, device starts
+      using buffers in the active leak queue.
+\item Upon detecting that buffers have been used, driver
+      switches to another leak queue making it active
+      (e.g. from \field{leakq1} to \field{leakq2} or vice versa).
+      It then starts adding buffers to the new leak queue.
+\item Device will keep using buffers in the active leak queue
+      until it detects that both the current leak queue is empty and another
+      leak queue has buffers. At that point device switches to
+      another leak queue, making it active.
+\item After the switch, buffers from the new leak queue are not
+      used until an information leak is detected.
+\end{enumerate}
--

MST

Hi Michael,

And thanks a lot on the effort on that.

Quoted 95 lines · Michael S. Tsirkin

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.

Add virtqueues to support reporting entropy leaks (similar to virtio based vmgenid).

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  virtio-rng.tex | 61 +++++++++++++++++++++++++++++++++++++++++++++++++-

  1 file changed, 60 insertions(+), 1 deletion(-)

virtio-rng.tex +0 -0

index 1ec7164..4760dfa 100644
@@ -9,10 +9,14 @@ \subsection{Device ID}\label{sec:Device Types / 
 Entropy Device / Device ID}
   \subsection{Virtqueues}\label{sec:Device Types / Entropy Device / Virtqueues}
   \begin{description}

  \item[0] requestq
+\item[1] leakq1 (only if VIRTIO_RNG_F_LEAK is offered) +\item[2] leakq2 (only if VIRTIO_RNG_F_LEAK is offered) \end{description}

  \subsection{Feature bits}\label{sec:Device Types / Entropy Device / Feature bits} -  None currently defined
+\begin{description}
+\item[VIRTIO_RNG_F_LEAK (0)] Device can report and handle
information leaks.
+\end{description}

  \subsection{Device configuration layout}\label{sec:Device Types / Entropy Device / Device configuration layout} None currently defined.

virtio-rng.tex +0 -0

@@ -21,6 +25,7 @@ \subsection{Device Initialization}\label{sec:Device 

Types / Entropy Device / Dev

  \begin{enumerate}
  \item The \field{requestq} virtqueue is initialized +\item If VIRTIO_RNG_F_LEAK has been negotiated, \field{leakq1} and \field{leakq2} are initialized
  \end{enumerate}

  \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device Operation}

virtio-rng.tex +0 -0

@@ -41,3 +46,57 @@ \subsection{Device Operation}\label{sec:Device 

Types / Entropy Device / Device O
  The device MUST place one or more random bytes into the buffer made available to it through \field{requestq}, but it MAY use less than the entire buffer length.
+
+\subsubsection{Reporting Information Leaks}{Device Types / Entropy Device / Device Operation / Reporting Information Leaks}

virtio-rng.tex +23 -0

+
+The device might, after the fact, detect that some of the entropy
+it supplied to the driver has after the fact degraded in quality
+or leaked to the outside world.  One example is when the device
+is part of the virtual machine undergoing a restore from snapshot
+operation. Another example is when the information leaks from the
+host system through a side-channel.
+
+The driver would typically react by causing regeneration of any
+information that might have leaked and that has to be secret or
+unique.  It is understood that when an information leak has been
+detected it is likely not limited to the entropy received through
+the specific device. In particular, this is the case for
+snapshoting It is thus suggested that the system fully
+regenerate any unique/secret information in this scenario.
+
+If VIRTIO_RNG_F_LEAK has been negotiated the device can report
+such leaks to the driver through a set of dedicated leak
+queues: \field{leakq1} and \field{leakq2}.
+
+Buffers added to the leak queues can have one of two forms:
+\begin{enumerate}
+\item A write-only buffer. It will be completely filled by random 

data by the device.
+\item A buffer consisting of read-only section followed by a

virtio-rng.tex +9 -0

+write-only section, both of identical size. The
+device will copy data from the read-only section to the write-only
+section.
+\end{enumerate}
+
+The steps for operating the virtqueue are:
+
+\begin{enumerate}
+\item At each time, only one of \field{leakq1}, \field{leakq2} is 

active
+      (has buffers added/used).
+\item After initialization, \field{leakq1} is active. +\item Driver adds multiple buffers to the active leak queue. +\item The buffers are not used until an information leak is +      detected, as long as that is the case driver can +      add more buffers to the active queue.
+\item Upon detecting an information leak, device starts +      using buffers in the active leak queue. +\item Upon detecting that buffers have been used, driver

Quoted 8 lines

+      switches to another leak queue making it active
+      (e.g. from \field{leakq1} to \field{leakq2} or vice versa). +      It then starts adding buffers to the new leak queue. +\item Device will keep using buffers in the active leak queue +      until it detects that both the current leak queue is empty and another
+      leak queue has buffers. At that point device switches to +      another leak queue, making it active.

Quoted 6 lines

+\item After the switch, buffers from the new leak queue are not +      used until an information leak is detected. +\end{enumerate}

Signature · 1 line

MST

I am taking now a closer look to trying to see any potential race-conditions but in a first glance it looks ok to me. Curious to see what Jason thinks too.

Cheers,
Babis
Amazon Spain Services sociedad limitada unipersonal, Calle Ramirez de Prado 5, 28045 Madrid. Registro Mercantil de Madrid . Tomo 22458 . Folio 102 . Hoja M-401234 . CIF B84570936

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.

Add virtqueues to support reporting entropy leaks (similar to virtio based vmgenid).

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  virtio-rng.tex | 61 +++++++++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 60 insertions(+), 1 deletion(-)

virtio-rng.tex +60 -2

index 1ec7164..4760dfa 100644
@@ -9,10 +9,14 @@ \subsection{Device ID}\label{sec:Device Types / Entropy Device / Device ID}
  \subsection{Virtqueues}\label{sec:Device Types / Entropy Device / Virtqueues}
  \begin{description}
  \item[0] requestq
+\item[1] leakq1 (only if VIRTIO_RNG_F_LEAK is offered)
+\item[2] leakq2 (only if VIRTIO_RNG_F_LEAK is offered)
  \end{description}
 
  \subsection{Feature bits}\label{sec:Device Types / Entropy Device / Feature bits}
-  None currently defined
+\begin{description}
+\item[VIRTIO_RNG_F_LEAK (0)] Device can report and handle information leaks.
+\end{description}
 
  \subsection{Device configuration layout}\label{sec:Device Types / Entropy Device / Device configuration layout}
    None currently defined.

@@ -21,6 +25,7 @@ \subsection{Device Initialization}\label{sec:Device Types / Entropy Device / Dev
 
  \begin{enumerate}
  \item The \field{requestq} virtqueue is initialized
+\item If VIRTIO_RNG_F_LEAK has been negotiated, \field{leakq1} and \field{leakq2} are initialized
  \end{enumerate}
 
  \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device Operation}

@@ -41,3 +46,57 @@ \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O
  The device MUST place one or more random bytes into the buffer
  made available to it through \field{requestq}, but it
  MAY use less than the entire buffer length.
+
+\subsubsection{Reporting Information Leaks}{Device Types / Entropy Device / Device Operation / Reporting Information Leaks}
+
+The device might, after the fact, detect that some of the entropy
+it supplied to the driver has after the fact degraded in quality
+or leaked to the outside world.  One example is when the device
+is part of the virtual machine undergoing a restore from snapshot
+operation. Another example is when the information leaks from the
+host system through a side-channel.
+
+The driver would typically react by causing regeneration of any
+information that might have leaked and that has to be secret or
+unique.  It is understood that when an information leak has been
+detected it is likely not limited to the entropy received through
+the specific device. In particular, this is the case for
+snapshoting It is thus suggested that the system fully
+regenerate any unique/secret information in this scenario.
+
+If VIRTIO_RNG_F_LEAK has been negotiated the device can report
+such leaks to the driver through a set of dedicated leak
+queues: \field{leakq1} and \field{leakq2}.
+
+Buffers added to the leak queues can have one of two forms:
+\begin{enumerate}
+\item A write-only buffer. It will be completely filled by random data by the device.
+\item A buffer consisting of read-only section followed by a
+write-only section, both of identical size. The
+device will copy data from the read-only section to the write-only
+section.
+\end{enumerate}
+
+The steps for operating the virtqueue are:
+
+\begin{enumerate}
+\item At each time, only one of \field{leakq1}, \field{leakq2} is active
+      (has buffers added/used).
+\item After initialization, \field{leakq1} is active.
+\item Driver adds multiple buffers to the active leak queue.
+\item The buffers are not used until an information leak is
+      detected, as long as that is the case driver can
+      add more buffers to the active queue.
+\item Upon detecting an information leak, device starts
+      using buffers in the active leak queue.
+\item Upon detecting that buffers have been used, driver
+      switches to another leak queue making it active
+      (e.g. from \field{leakq1} to \field{leakq2} or vice versa).
+      It then starts adding buffers to the new leak queue.
+\item Device will keep using buffers in the active leak queue
+      until it detects that both the current leak queue is empty and another
+      leak queue has buffers. At that point device switches to
+      another leak queue, making it active.
+\item After the switch, buffers from the new leak queue are not
+      used until an information leak is detected.
+\end{enumerate}
--

MST

Add virtqueues to support reporting entropy leaks (similar to virtio based vmgenid).

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  virtio-rng.tex | 61 +++++++++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 60 insertions(+), 1 deletion(-)

virtio-rng.tex +43 -1

index 1ec7164..4760dfa 100644
@@ -9,10 +9,14 @@ \subsection{Device ID}\label{sec:Device Types / Entropy Device / Device ID}
  \subsection{Virtqueues}\label{sec:Device Types / Entropy Device / Virtqueues}
  \begin{description}
  \item[0] requestq
+\item[1] leakq1 (only if VIRTIO_RNG_F_LEAK is offered)
+\item[2] leakq2 (only if VIRTIO_RNG_F_LEAK is offered)
  \end{description}
 
  \subsection{Feature bits}\label{sec:Device Types / Entropy Device / Feature bits}
-  None currently defined
+\begin{description}
+\item[VIRTIO_RNG_F_LEAK (0)] Device can report and handle information leaks.
+\end{description}
 
  \subsection{Device configuration layout}\label{sec:Device Types / Entropy Device / Device configuration layout}
    None currently defined.

@@ -21,6 +25,7 @@ \subsection{Device Initialization}\label{sec:Device Types / Entropy Device / Dev
 
  \begin{enumerate}
  \item The \field{requestq} virtqueue is initialized
+\item If VIRTIO_RNG_F_LEAK has been negotiated, \field{leakq1} and \field{leakq2} are initialized
  \end{enumerate}
 
  \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device Operation}

@@ -41,3 +46,57 @@ \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O
  The device MUST place one or more random bytes into the buffer
  made available to it through \field{requestq}, but it
  MAY use less than the entire buffer length.
+
+\subsubsection{Reporting Information Leaks}{Device Types / Entropy Device / Device Operation / Reporting Information Leaks}
+
+The device might, after the fact, detect that some of the entropy
+it supplied to the driver has after the fact degraded in quality
+or leaked to the outside world.  One example is when the device
+is part of the virtual machine undergoing a restore from snapshot
+operation. Another example is when the information leaks from the
+host system through a side-channel.
+
+The driver would typically react by causing regeneration of any
+information that might have leaked and that has to be secret or
+unique.  It is understood that when an information leak has been
+detected it is likely not limited to the entropy received through
+the specific device. In particular, this is the case for
+snapshoting It is thus suggested that the system fully
+regenerate any unique/secret information in this scenario.
+
+If VIRTIO_RNG_F_LEAK has been negotiated the device can report
+such leaks to the driver through a set of dedicated leak
+queues: \field{leakq1} and \field{leakq2}.
+
+Buffers added to the leak queues can have one of two forms:
+\begin{enumerate}
+\item A write-only buffer. It will be completely filled by random data by the device.
+\item A buffer consisting of read-only section followed by a
+write-only section, both of identical size. The
+device will copy data from the read-only section to the write-only
+section.
+\end{enumerate}
+
+The steps for operating the virtqueue are:
+
+\begin{enumerate}
+\item At each time, only one of \field{leakq1}, \field{leakq2} is active
+      (has buffers added/used).
+\item After initialization, \field{leakq1} is active.

Hi Michael,

Quoted 81 lines · Michael S. Tsirkin

Add virtqueues to support reporting entropy leaks (similar to virtio based vmgenid).

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  virtio-rng.tex | 61 +++++++++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 60 insertions(+), 1 deletion(-)

virtio-rng.tex +52 -1

index 1ec7164..4760dfa 100644
@@ -9,10 +9,14 @@ \subsection{Device ID}\label{sec:Device Types / Entropy Device / Device ID}
  \subsection{Virtqueues}\label{sec:Device Types / Entropy Device / Virtqueues}
  \begin{description}
  \item[0] requestq
+\item[1] leakq1 (only if VIRTIO_RNG_F_LEAK is offered)
+\item[2] leakq2 (only if VIRTIO_RNG_F_LEAK is offered)
  \end{description}
  \subsection{Feature bits}\label{sec:Device Types / Entropy Device / Feature bits}
-  None currently defined
+\begin{description}
+\item[VIRTIO_RNG_F_LEAK (0)] Device can report and handle information leaks.
+\end{description}
  \subsection{Device configuration layout}\label{sec:Device Types / Entropy Device / Device configuration layout}
    None currently defined.

@@ -21,6 +25,7 @@ \subsection{Device Initialization}\label{sec:Device Types / Entropy Device / Dev
  \begin{enumerate}
  \item The \field{requestq} virtqueue is initialized
+\item If VIRTIO_RNG_F_LEAK has been negotiated, \field{leakq1} and \field{leakq2} are initialized
  \end{enumerate}
  \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device Operation}

@@ -41,3 +46,57 @@ \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O
  The device MUST place one or more random bytes into the buffer
  made available to it through \field{requestq}, but it
  MAY use less than the entire buffer length.
+
+\subsubsection{Reporting Information Leaks}{Device Types / Entropy Device / Device Operation / Reporting Information Leaks}
+
+The device might, after the fact, detect that some of the entropy
+it supplied to the driver has after the fact degraded in quality
+or leaked to the outside world.  One example is when the device
+is part of the virtual machine undergoing a restore from snapshot
+operation. Another example is when the information leaks from the
+host system through a side-channel.
+
+The driver would typically react by causing regeneration of any
+information that might have leaked and that has to be secret or
+unique.  It is understood that when an information leak has been
+detected it is likely not limited to the entropy received through
+the specific device. In particular, this is the case for
+snapshoting It is thus suggested that the system fully
+regenerate any unique/secret information in this scenario.
+
+If VIRTIO_RNG_F_LEAK has been negotiated the device can report
+such leaks to the driver through a set of dedicated leak
+queues: \field{leakq1} and \field{leakq2}.
+
+Buffers added to the leak queues can have one of two forms:
+\begin{enumerate}
+\item A write-only buffer. It will be completely filled by random data by the device.
+\item A buffer consisting of read-only section followed by a
+write-only section, both of identical size. The
+device will copy data from the read-only section to the write-only
+section.
+\end{enumerate}
+
+The steps for operating the virtqueue are:
+
+\begin{enumerate}
+\item At each time, only one of \field{leakq1}, \field{leakq2} is active
+      (has buffers added/used).
+\item After initialization, \field{leakq1} is active.
+\item Driver adds multiple buffers to the active leak queue.
+\item The buffers are not used until an information leak is
+      detected, as long as that is the case driver can
+      add more buffers to the active queue.
+\item Upon detecting an information leak, device starts
+      using buffers in the active leak queue.
+\item Upon detecting that buffers have been used, driver
+      switches to another leak queue making it active
+      (e.g. from \field{leakq1} to \field{leakq2} or vice versa).

virtio-rng.tex \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O · line 95

Quoted 1 line

virtio-rng.tex +1 -0

+      It then starts adding buffers to the new leak queue.\

I have been discussing with Alex and we think there's a potential race here, between the time the driver
sees the used buffers in the active leak queue until the time it adds new buffers to the next leak queue.
If a new entropy leak event arrives the VMM won't find any buffers in the queue.

In the last RFC implementing this in Linux we sent to LKML [1] we avoid the issue by pre-populating both
queues, but that does not solve the problem if a third entropy leak event arrives. The probability of this
happening is indeed small, but we thought of a potential solution to this.

What if we modify the spec here to instruct the VMM to deny taking a snapshot if there are not any buffers
in the active leak queue? If we did this, we could even simplify the spec to just introduce a single entropy
leak queue, so we could avoid the complexity of switching between active leak queues in the driver and
the device. WDYT?

Cheers,
Babis

[1]
https://lore.kernel.org/lkml/[email protected]/T/

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

I do not understand why this matters though. we know there was a leak, why does it matter whether there was one or two leaks?

Quoted 6 lines

In the last RFC implementing this in Linux we sent to LKML [1] we avoid the issue by pre-populating both
queues, but that does not solve the problem if a third entropy leak event arrives. The probability of this
happening is indeed small, but we thought of a potential solution to this.

Quoted 7 lines

What if we modify the spec here to instruct the VMM to deny taking a snapshot if there are not any buffers
in the active leak queue? If we did this, we could even simplify the spec to just introduce a single entropy
leak queue, so we could avoid the complexity of switching between active leak queues in the driver and
the device. WDYT?

here's the problem:

- driver adds batch 1 of buffers
- leak
- device starts using buffers from batch 1
- driver sees some buffers and starts adding batch 2

Quoted 4 lines

+\item Upon detecting that buffers have been used, driver +      switches to another leak queue making it active +      (e.g. from \field{leakq1} to \field{leakq2} or vice versa). +      It then starts adding buffers to the new leak queue.

Quoted 7 lines

+\item Device will keep using buffers in the active leak queue
+      until it detects that both the current leak queue is empty and another +      leak queue has buffers. At that point device switches to
+      another leak queue, making it active.
+\item After the switch, buffers from the new leak queue are not +      used until an information leak is detected. +\end{enumerate}

- device sees batch 2 and thinks this is part of batch 1 consumes them all

Quoted 4 lines

I do not understand why this matters though. we know there was a leak, why does it matter whether there was one or two leaks?

Quoted 1 line

In the last RFC implementing this in Linux we sent to LKML [1] we avoid

the

Quoted 2 lines

Quoted 2 lines

issue by pre-populating both
queues, but that does not solve the problem if a third entropy leak

event

Quoted 2 lines

Quoted 2 lines

arrives. The probability of this
happening is indeed small, but we thought of a potential solution to

this.

Quoted 4 lines

Quoted 4 lines

What if we modify the spec here to instruct the VMM to deny taking a snapshot if there are not any buffers
in the active leak queue? If we did this, we could even simplify the

spec to

Quoted 11 lines

Quoted 4 lines

just introduce a single entropy
leak queue, so we could avoid the complexity of switching between active leak queues in the driver and
the device. WDYT?

here's the problem:

- driver adds batch 1 of buffers
- leak
- device starts using buffers from batch 1
- driver sees some buffers and starts adding batch 2

If understand this clause:

Quoted 4 lines

Quoted 4 lines

+\item Upon detecting that buffers have been used, driver +      switches to another leak queue making it active +      (e.g. from \field{leakq1} to \field{leakq2} or vice versa). +      It then starts adding buffers to the new leak queue.

correctly:

At this point, the driver will first switch active leak queue and then add batch 2 to the new leak queue.

and due to this:

Quoted 2 lines

Quoted 2 lines

+\item Device will keep using buffers in the active leak queue +      until it detects that both the current leak queue is empty and

another

Quoted 5 lines

Quoted 5 lines

+      leak queue has buffers. At that point device switches to +      another leak queue, making it active.
+\item After the switch, buffers from the new leak queue are not +      used until an information leak is detected. +\end{enumerate}

the following won't happen:

Quoted 2 lines

- device sees batch 2 and thinks this is part of batch 1 consumes them all

Does it make sense?

Cheers,
Babis

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.

Quoted 48 lines · Babis Chalios

Quoted 4 lines

I do not understand why this matters though. we know there was a leak, why does it matter whether there was one or two leaks?

Quoted 1 line

In the last RFC implementing this in Linux we sent to LKML [1] we avoid

the

Quoted 2 lines

Quoted 2 lines

issue by pre-populating both
queues, but that does not solve the problem if a third entropy leak

event

Quoted 2 lines

Quoted 2 lines

arrives. The probability of this
happening is indeed small, but we thought of a potential solution to

this.

Quoted 3 lines

Quoted 3 lines

What if we modify the spec here to instruct the VMM to deny taking a snapshot if there are not any buffers
in the active leak queue? If we did this, we could even simplify the

spec to

Quoted 10 lines

Quoted 4 lines

just introduce a single entropy
leak queue, so we could avoid the complexity of switching between active leak queues in the driver and
the device. WDYT?

here's the problem:

- driver adds batch 1 of buffers
- leak
- device starts using buffers from batch 1
- driver sees some buffers and starts adding batch 2

If understand this clause:

Quoted 4 lines

virtio-rng.tex \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O · line 95

Quoted 4 lines

+\item Upon detecting that buffers have been used, driver

virtio-rng.tex +3 -0

+      switches to another leak queue making it active
+      (e.g. from \field{leakq1} to \field{leakq2} or vice versa).
+      It then starts adding buffers to the new leak queue.

correctly:

At this point, the driver will first switch active leak queue and then add batch 2 to the new leak queue.

and due to this:

Quoted 2 lines

Quoted 2 lines

+\item Device will keep using buffers in the active leak queue
+ until it detects that both the current leak queue is empty and

another

Quoted 5 lines

virtio-rng.tex \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O · line 102

Quoted 5 lines

virtio-rng.tex +5 -0

+      leak queue has buffers. At that point device switches to
+      another leak queue, making it active.
+\item After the switch, buffers from the new leak queue are not
+      used until an information leak is detected.
+\end{enumerate}

the following won't happen:

Quoted 6 lines

Quoted 2 lines

- device sees batch 2 and thinks this is part of batch 1 consumes them all

Does it make sense?

Cheers,
Babis

yes, the queue switch is used as a barrier to detect a new leak event.

--
MST

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Quoted 2 lines

yes, the queue switch is used as a barrier to detect a new leak event.

Right, so I think that there is a race condition between the time the driver sees the used buffers of the first batch and until it adds the second batch on the next leak queue.

1. driver adds batch 1
2. leak event
3. device uses batch 1
4. driver sees the used buffers and
    a. switches leak queues
    b. adds batch 2.
5. devices finds initial leak queue empty and sees buffers in second leak queue.

If a second leak event happens after step 3 above and before all of steps 4 complete then batch 2 will not be processed as part of the second leak event.

Quoted 53 lines · Michael S. Tsirkin

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.

Quoted 48 lines · Babis Chalios

Quoted 4 lines

I do not understand why this matters though. we know there was a leak, why does it matter whether there was one or two leaks?

Quoted 1 line

In the last RFC implementing this in Linux we sent to LKML [1] we avoid

the

Quoted 2 lines

Quoted 2 lines

issue by pre-populating both
queues, but that does not solve the problem if a third entropy leak

event

Quoted 2 lines

Quoted 2 lines

arrives. The probability of this
happening is indeed small, but we thought of a potential solution to

this.

Quoted 3 lines

Quoted 3 lines

What if we modify the spec here to instruct the VMM to deny taking a snapshot if there are not any buffers
in the active leak queue? If we did this, we could even simplify the

spec to

Quoted 10 lines

Quoted 4 lines

just introduce a single entropy
leak queue, so we could avoid the complexity of switching between active leak queues in the driver and
the device. WDYT?

here's the problem:

- driver adds batch 1 of buffers
- leak
- device starts using buffers from batch 1
- driver sees some buffers and starts adding batch 2

If understand this clause:

Quoted 4 lines

virtio-rng.tex \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O · line 95

Quoted 4 lines

+\item Upon detecting that buffers have been used, driver

virtio-rng.tex +3 -0

+      switches to another leak queue making it active
+      (e.g. from \field{leakq1} to \field{leakq2} or vice versa).
+      It then starts adding buffers to the new leak queue.

correctly:

At this point, the driver will first switch active leak queue and then add batch 2 to the new leak queue.

and due to this:

Quoted 2 lines

Quoted 2 lines

+\item Device will keep using buffers in the active leak queue
+ until it detects that both the current leak queue is empty and

another

Quoted 5 lines

virtio-rng.tex \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O · line 102

Quoted 5 lines

virtio-rng.tex +5 -0

+      leak queue has buffers. At that point device switches to
+      another leak queue, making it active.
+\item After the switch, buffers from the new leak queue are not
+      used until an information leak is detected.
+\end{enumerate}

the following won't happen:

Quoted 7 lines

Quoted 6 lines

Quoted 2 lines

- device sees batch 2 and thinks this is part of batch 1 consumes them all

Does it make sense?

Cheers,
Babis

yes, the queue switch is used as a barrier to detect a new leak event.

Right, so I think that there is a race condition between the time the driver sees the used buffers of the first
batch and until it adds the second batch on the next leak queue.

1. driver adds batch 1
2. leak event
3. device uses batch 1
4. driver sees the used buffers and
    a. switches leak queues
    b. adds batch 2.
5. devices finds initial leak queue empty and sees buffers in second leak queue.

If a second leak event happens after step 3 above and before all of steps 4 complete then batch 2 will not
be processed as part of the second leak event.

Quoted 8 lines

--
MST

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Quoted 14 lines

Right, so I think that there is a race condition between the time the driver sees the used buffers of the first
batch and until it adds the second batch on the next leak queue.

1. driver adds batch 1
2. leak event
3. device uses batch 1
4. driver sees the used buffers and
a. switches leak queues
b. adds batch 2.
5. devices finds initial leak queue empty and sees buffers in second leak queue.

Quoted 3 lines

If a second leak event happens after step 3 above and before all of steps 4 complete then batch 2 will not
be processed as part of the second leak event.

driver can just pre-add buffers in the second queue.

1. available buffers to queue 1-X
2. available buffers to queue X

3. poll queue X
4. used buffers in queue X
5. avail buffers in queue X
6. poll queue 1-X
7. used buffers in queue X
8. avail buffers in queue X
9. goto 3

Quoted 23 lines

Quoted 14 lines

Right, so I think that there is a race condition between the time the driver sees the used buffers of the first
batch and until it adds the second batch on the next leak queue.

1. driver adds batch 1
2. leak event
3. device uses batch 1
4. driver sees the used buffers and
a. switches leak queues
b. adds batch 2.
5. devices finds initial leak queue empty and sees buffers in second leak queue.

Quoted 3 lines

If a second leak event happens after step 3 above and before all of steps 4 complete then batch 2 will not
be processed as part of the second leak event.

driver can just pre-add buffers in the second queue.

1. available buffers to queue 1-X
2. available buffers to queue X

Quoted 8 lines

3. poll queue X
4. used buffers in queue X
5. avail buffers in queue X
6. poll queue 1-X
7. used buffers in queue X
8. avail buffers in queue X
9. goto 3

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

If, instead, we define a single leak queue and require that VMM should refuse to take a snapshot if that queue is empty, we avoid the race condition in all cases and IMHO the protocol becomes much simpler.

Cheers,
Babis

Quoted 40 lines · Babis Chalios

Quoted 22 lines

Quoted 13 lines

Right, so I think that there is a race condition between the time the driver sees the used buffers of the first
batch and until it adds the second batch on the next leak queue.

1. driver adds batch 1
2. leak event
3. device uses batch 1
4. driver sees the used buffers and
a. switches leak queues
b. adds batch 2.
5. devices finds initial leak queue empty and sees buffers in second leak queue.

Quoted 3 lines

If a second leak event happens after step 3 above and before all of steps 4 complete then batch 2 will not
be processed as part of the second leak event.

driver can just pre-add buffers in the second queue.

1. available buffers to queue 1-X
2. available buffers to queue X

Quoted 8 lines

3. poll queue X
4. used buffers in queue X
5. avail buffers in queue X
6. poll queue 1-X
7. used buffers in queue X
8. avail buffers in queue X
9. goto 3

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

Quoted 8 lines

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.

Quoted 7 lines

If, instead, we define a single leak queue and require that VMM should refuse to take a snapshot if that queue is empty, we avoid the race condition in all cases and IMHO the protocol becomes much simpler.

Quoted 3 lines

Cheers,
Babis

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Quoted 50 lines · Michael S. Tsirkin

Quoted 48 lines · Babis Chalios

Quoted 22 lines

Quoted 13 lines

Right, so I think that there is a race condition between the time the driver sees the used buffers of the first
batch and until it adds the second batch on the next leak queue.

1. driver adds batch 1
2. leak event
3. device uses batch 1
4. driver sees the used buffers and
a. switches leak queues
b. adds batch 2.
5. devices finds initial leak queue empty and sees buffers in second leak queue.

Quoted 3 lines

If a second leak event happens after step 3 above and before all of steps 4 complete then batch 2 will not
be processed as part of the second leak event.

driver can just pre-add buffers in the second queue.

1. available buffers to queue 1-X
2. available buffers to queue X

Quoted 8 lines

3. poll queue X
4. used buffers in queue X
5. avail buffers in queue X
6. poll queue 1-X
7. used buffers in queue X
8. avail buffers in queue X
9. goto 3

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

Quoted 1 line

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?

Quoted 8 lines

Quoted 8 lines

If, instead, we define a single leak queue and require that VMM should refuse to take a snapshot if that queue is empty, we avoid the race condition in all cases and IMHO the protocol becomes much simpler.

Quoted 8 lines

Quoted 3 lines

Cheers,
Babis

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Quoted 18 lines

Quoted 18 lines

Quoted 18 lines

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

Quoted 4 lines

Quoted 1 line

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.

Quoted 18 lines

Quoted 18 lines

Quoted 18 lines

Quoted 18 lines

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

Quoted 8 lines

Quoted 4 lines

Quoted 1 line

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.

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 24 lines · Babis Chalios

Quoted 24 lines

Quoted 24 lines

Quoted 24 lines

Quoted 12 lines

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

Quoted 11 lines

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.

Quoted 8 lines

Quoted 8 lines

Quoted 4 lines

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

Quoted 62 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 3 lines

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?

Quoted 63 lines · Michael S. Tsirkin

Quoted 62 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 3 lines

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.

Quoted 63 lines · Michael S. Tsirkin

Quoted 62 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 3 lines

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

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Quoted 78 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 3 lines

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).

Quoted 5 lines

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.

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?

Quoted 79 lines · Michael S. Tsirkin

Quoted 78 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 3 lines

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).

Quoted 7 lines

Quoted 5 lines

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.

Quoted 4 lines

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

Quoted 96 lines · Babis Chalios

Quoted 79 lines · Michael S. Tsirkin

Quoted 78 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 3 lines

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).

Quoted 7 lines

Quoted 5 lines

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.

Quoted 4 lines

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?

Quoted 5 lines

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.

Quoted 103 lines · Michael S. Tsirkin

Quoted 96 lines · Babis Chalios

Quoted 79 lines · Michael S. Tsirkin

Quoted 78 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 3 lines

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).

Quoted 7 lines

Quoted 5 lines

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.

Quoted 4 lines

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?

Quoted 5 lines

I am not familiar with MAP/UNMAP descriptors. Is there a link where I can read about them?

Cheers,
Babis

Quoted 6 lines

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

Quoted 117 lines · Babis Chalios

Quoted 99 lines · Michael S. Tsirkin

Quoted 98 lines · Babis Chalios

Quoted 79 lines · Michael S. Tsirkin

Quoted 78 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 3 lines

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).

Quoted 7 lines

Quoted 5 lines

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.

Quoted 4 lines

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?

Quoted 8 lines

Quoted 2 lines

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.

Quoted 2 lines

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?

Quoted 118 lines · Michael S. Tsirkin

Quoted 117 lines · Babis Chalios

Quoted 99 lines · Michael S. Tsirkin

Quoted 98 lines · Babis Chalios

Quoted 79 lines · Michael S. Tsirkin

Quoted 78 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 3 lines

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).

Quoted 7 lines

Quoted 5 lines

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.

Quoted 4 lines

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?

Quoted 8 lines

Quoted 2 lines

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.

Quoted 7 lines

Quoted 2 lines

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

Quoted 146 lines · Babis Chalios

Quoted 130 lines · Michael S. Tsirkin

Quoted 129 lines · Babis Chalios

Quoted 111 lines · Michael S. Tsirkin

Quoted 110 lines · Babis Chalios

Quoted 90 lines · Michael S. Tsirkin

Quoted 89 lines · Babis Chalios

Quoted 70 lines · Michael S. Tsirkin

Quoted 69 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 28 lines · Babis Chalios

Quoted 28 lines

Quoted 28 lines

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?

Quoted 5 lines

Quoted 5 lines

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?

Quoted 8 lines

Quoted 3 lines

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).

Quoted 8 lines

Quoted 6 lines

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.

Quoted 4 lines

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?

Quoted 8 lines

Quoted 2 lines

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.

Quoted 7 lines

Quoted 2 lines

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.

Quoted 7 lines

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

Hey Michael, does this explain the flow of things?

To summarize, end-to-end things work like this:

1. kernel allocates a page where an epoch value is stored.
2. User-space can mmap this page and look for changes in its value to know when it needs to re-seed its PRNGs.
3. virtio-rng driver gets a hold of the address of the page and programs device to update it when entropy is leaked
4. Other kernel sub-systems can do the same if they have a concept of entropy leaking. For example, random.c might want to do this periodically

Regarding your suggestion for MAP/UNMAP buffers, I think it could work if it was working with kernel addresses as well. The device doesn't care about user vs kernel space guest addresses after all. However, if we were to use such a mechanism, it seems that we don't need the two leak queues anyway.

So, it sounds like we can have a setup with one leak queue only. If we use a MAP/UNMAP type of buffer then all is fine. If we use normal buffers, we can add the requirement that the VMM needs to refuse to take a snapshot if there is not a copy on leak buffer in the queue.

Thoughts?

Cheers,
Babis

Quoted 118 lines · Michael S. Tsirkin

Quoted 117 lines · Babis Chalios

Quoted 99 lines · Michael S. Tsirkin

Quoted 98 lines · Babis Chalios

Quoted 79 lines · Michael S. Tsirkin

Quoted 78 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 3 lines

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).

Quoted 7 lines

Quoted 5 lines

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.

Quoted 4 lines

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?

Quoted 8 lines

Quoted 2 lines

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.

Quoted 7 lines

Quoted 2 lines

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

Quoted 136 lines · Babis Chalios

Quoted 118 lines · Michael S. Tsirkin

Quoted 117 lines · Babis Chalios

Quoted 99 lines · Michael S. Tsirkin

Quoted 98 lines · Babis Chalios

Quoted 79 lines · Michael S. Tsirkin

Quoted 78 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

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?

Quoted 8 lines

Quoted 3 lines

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).

Quoted 7 lines

Quoted 5 lines

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.

Quoted 4 lines

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?

Quoted 8 lines

Quoted 2 lines

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.

Quoted 6 lines

Quoted 2 lines

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.

Quoted 2 lines

Cheers,
Babis

This idea would be fine but I don't see how it's so different from VMGENID. Care to explain?

Quoted 141 lines · Michael S. Tsirkin

Quoted 138 lines · Babis Chalios

Quoted 118 lines · Michael S. Tsirkin

Quoted 117 lines · Babis Chalios

Quoted 99 lines · Michael S. Tsirkin

Quoted 98 lines · Babis Chalios

Quoted 79 lines · Michael S. Tsirkin

Quoted 78 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

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?

Quoted 8 lines

Quoted 3 lines

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).

Quoted 7 lines

Quoted 5 lines

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.

Quoted 4 lines

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?

Quoted 8 lines

Quoted 2 lines

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.

Quoted 6 lines

Quoted 2 lines

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

Quoted 3 lines

This idea would be fine but I don't see how it's so different from VMGENID. Care to explain?

It is different in that the memory is owned by the guest kernel, not the hardware. In this case, random.c maintains it. This allows, systems other than the hardware, e.g. virtio-rng to notify about entropy leak events. For example, random.c itself can periodically do that (I think Jason had that use-case in mind).

The fact that we as well mmap that memory to the user-space is just for giving user space a mechanism that allows it to know when it needs to reseed its PRNGs. We _could_ have done the same with VMGENID, but the fact that, in this case, the underlying physical memory is owned by the ACPI device blocks us from letting other systems sending notifications
as well.

Maybe it makes sense to have this discussion in the patch we sent for Linux on LKML [1]?

Cheers,
Babis

[1] https://lore.kernel.org/lkml/[email protected]/T/#mb1242999d8296169d9a4ee1a0805005633ec146a

Quoted 142 lines · Babis Chalios

Quoted 141 lines · Michael S. Tsirkin

Quoted 130 lines · Babis Chalios

Quoted 118 lines · Michael S. Tsirkin

Quoted 117 lines · Babis Chalios

Quoted 99 lines · Michael S. Tsirkin

Quoted 98 lines · Babis Chalios

Quoted 79 lines · Michael S. Tsirkin

Quoted 78 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 31 lines · Babis Chalios

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?

Quoted 8 lines

Quoted 3 lines

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).

Quoted 7 lines

Quoted 5 lines

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.

Quoted 4 lines

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?

Quoted 8 lines

Quoted 2 lines

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.

Quoted 6 lines

Quoted 2 lines

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.

Quoted 7 lines

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

This idea would be fine but I don't see how it's so different from VMGENID. Care to explain?

Quoted 5 lines

It is different in that the memory is owned by the guest kernel, not the hardware. In this case, random.c maintains it. This allows, systems other than the hardware, e.g. virtio-rng to notify about entropy leak events. For example, random.c itself can periodically do that (I think Jason had that use-case in mind).

No sure I understand. Example?

Quoted 11 lines

The fact that we as well mmap that memory to the user-space is just for giving user space a mechanism that allows it to know when it needs to reseed its PRNGs. We _could_ have done the same with VMGENID, but the fact that, in this case, the underlying physical memory is owned by the ACPI device blocks us from letting other systems sending notifications
as well.

Maybe it makes sense to have this discussion in the patch we sent for Linux on LKML [1]?

Quoted 4 lines

Cheers,
Babis

[1] https://lore.kernel.org/lkml/[email protected]/T/#mb1242999d8296169d9a4ee1a0805005633ec146a

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Quoted 82 lines · Michael S. Tsirkin

Quoted 81 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 3 lines

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.

Quoted 4 lines

Quoted 2 lines

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.

Quoted 4 lines

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

Quoted 84 lines · Babis Chalios

Quoted 82 lines · Michael S. Tsirkin

Quoted 81 lines · Babis Chalios

Quoted 62 lines · Michael S. Tsirkin

Quoted 61 lines · Babis Chalios

Resending to fix e-mail formatting issues (sorry for the spam)

Quoted 23 lines · Babis Chalios

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

Quoted 23 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 3 lines

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.

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

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.

OK. Basically I think if there's a way for device to detect that guest is stuck and not refilling the queue in a timely manner, then we are ok - host will make its own decisions on whether to snapshot or not.

However, I feel in that case we need a way to create a big backlog of buffers for guest to fill such that this ring empty condition is very unlikely.
One or even 2 queues does not seem enough then.

For example, I can see a "stop" command that will tell device: "stop consuming buffers" and device will stop consuming buffers until the next leak event.

Quoted 11 lines

Quoted 6 lines

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

Generally, entropy only grows. However, there are cases where it goes down - for example, consider generating a one time pad where someone managed to use a side channel to steal its contents. By combining the seemingly random pad with the stolen contents we have reversed the entropy.

This actually happens within VMs e.g. when time is reversed due to snapshoting. Existing approaches for VMs include Microsoft's VM GEN ID.

This draft proposes a feature in virtio rng for reporting such leaks.

Patches 1,2 refactor existing draft text. Patch 3 adds new functionality.

TODO:
document theory of operation
add conformance clauses

Michael S. Tsirkin (3):
rng: move to a file of its own
rng: be specific about the virtqueue
rng: leak detection support

 content.tex    |  43 +--------------------
 virtio-rng.tex | 102 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 42 deletions(-)
 create mode 100644 virtio-rng.tex

Signature · 1 line

MST

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.

Quoted 13 lines · Michael S. Tsirkin

Generally, entropy only grows. However, there are cases where it goes down - for example, consider generating a one time pad where someone managed to use a side channel to steal its contents. By combining the seemingly random pad with the stolen contents we have reversed the entropy.

This actually happens within VMs e.g. when time is reversed due to snapshoting. Existing approaches for VMs include Microsoft's VM GEN ID.

This draft proposes a feature in virtio rng for reporting such leaks.

Quoted 5 lines

Patches 1,2 refactor existing draft text. Patch 3 adds new functionality.

TODO:
document theory of operation
add conformance clauses

Guys any input on this? Anyone going to use this?

Quoted 10 lines

Michael S. Tsirkin (3):
rng: move to a file of its own
rng: be specific about the virtqueue
rng: leak detection support

  content.tex    |  43 +--------------------
  virtio-rng.tex | 102 +++++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 103 insertions(+), 42 deletions(-)

create mode 100644 virtio-rng.tex

Quoted 3 lines

--
MST

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Hey guys,

Just FYI, I am still interested in this, but it's currently taken a backseat while I focus on some other parts of the ecosystem. I'll be back to moving this forward in March.

Jason