Message-ID: <e1c03136-b873-1f1d-8b06-d9186566fc0c@amazon.es>
Date:   Mon, 13 Mar 2023 11:42:40 +0100
From:   <bchalios@...zon.es>
To:     Amit Shah <amit@...radead.org>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Olivia Mackall <olivia@...enic.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        "Jason Wang" <jasowang@...hat.com>, <linux-crypto@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <virtualization@...ts.linux-foundation.org>, <sgarzare@...hat.com>,
        <amit@...nel.org>, <graf@...zon.de>, <xmarcalx@...zon.co.uk>
Subject: Re: [PATCH v2 0/2] [RFC] virtio-rng entropy leak reporting feature

Hi Amit,

Thanks for taking the time to look into this.

On 3/2/23 5:55 PM, Amit Shah <amit@...radead.org> wrote:
> 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.
> 
> 
> 
> Hey all,
> 
> On Tue, 2023-01-31 at 17:27 +0100, Jason A. Donenfeld wrote:
> > You sent a v2, but I'm not back until the 11th to provide comments on
> > v1. I still think this isn't the right direction, as this needs tie-ins
> > to the rng to actually be useful. Please stop posting new versions of
> > this for now, so that somebody doesn't accidentally merge it; that'd be
> > a big mistake. I'll paste what I wrote you prior:
> 
> I sat down to review the patchset but looks like there's some
> background I'm not aware of.
> 
> It looks like Babis has implemented the guest side here according to
> the spec change that Michael has posted.
> 
> Jason, do you have an alternative in mind?  In that case, we should
> pick this up in the spec update thread instead.

I am not sure what Jason meant here by "This needs to be integrated more closely with random.c itself, similar to how vmgenid works",
but here's my take on this.

The point of the patchset is to provide an implementation of Michael's spec on which we can discuss. It implements the HW API and it has
some hooks showing how this API could be used. It is mainly directed towards the user-space where we did not have a proper API to consume
VMGENID-like functionality. With regards to the random.c components it does exactly what VMGENID does currently, i.e. whenever an entropy-leak
is detected it uses the new random bytes provided by the virtio-rng device as entropy. This is as racy as VMGENID, as I mention in the cover
letter of the patchset.

However, the new spec does allow us to do things _correctly_, i.e. not rely on asynchronous handling of events to re-seed the kernel. For example, we
could achieve something like that by making use of the "copy-on-leak" operation, so that a flag changes value before vCPUs get resumed, so we know
when a leak has happened when needed, e.g. before returning random bytes to user-space. At least, that's what I remember us discussing during LPC.
Jason, Michael, Alex, please keep me honest here :)

Unfortunately, I am not very familiar with the random.c code and did not want to do something there that would most certainly be wrong, hence I posted
this as an RFC, asking for input on how we could achieve this better integration. Hopefully, when Jason is back from his vacation he can share his thoughts
on this, but if yourself (or anyone else interested) have any ideas on how we could design this properly, I 'm happy to discuss!


> 
> Somehow it feels like I don't have the complete story for this feature
> set, having missed the discussion during LPC.
> 
>          Amit
> 


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

Powered by blists - more mailing lists