lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 23 Sep 2021 08:26:06 +0200 From: Laurent Vivier <lvivier@...hat.com> To: "Michael S. Tsirkin" <mst@...hat.com> Cc: linux-kernel@...r.kernel.org, Alexander Potapenko <glider@...gle.com>, linux-crypto@...r.kernel.org, Dmitriy Vyukov <dvyukov@...gle.com>, rusty@...tcorp.com.au, amit@...nel.org, akong@...hat.com, Herbert Xu <herbert@...dor.apana.org.au>, Matt Mackall <mpm@...enic.com>, virtualization@...ts.linux-foundation.org Subject: Re: [PATCH 1/4] hwrng: virtio - add an internal buffer On 22/09/2021 21:02, Michael S. Tsirkin wrote: > On Wed, Sep 22, 2021 at 07:09:00PM +0200, Laurent Vivier wrote: >> hwrng core uses two buffers that can be mixed in the >> virtio-rng queue. >> >> If the buffer is provided with wait=0 it is enqueued in the >> virtio-rng queue but unused by the caller. >> On the next call, core provides another buffer but the >> first one is filled instead and the new one queued. >> And the caller reads the data from the new one that is not >> updated, and the data in the first one are lost. >> >> To avoid this mix, virtio-rng needs to use its own unique >> internal buffer at a cost of a data copy to the caller buffer. >> >> Signed-off-by: Laurent Vivier <lvivier@...hat.com> >> --- >> drivers/char/hw_random/virtio-rng.c | 43 ++++++++++++++++++++++------- >> 1 file changed, 33 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c >> index a90001e02bf7..208c547dcac1 100644 >> --- a/drivers/char/hw_random/virtio-rng.c >> +++ b/drivers/char/hw_random/virtio-rng.c >> @@ -18,13 +18,20 @@ static DEFINE_IDA(rng_index_ida); >> struct virtrng_info { >> struct hwrng hwrng; >> struct virtqueue *vq; >> - struct completion have_data; >> char name[25]; >> - unsigned int data_avail; >> int index; >> bool busy; >> bool hwrng_register_done; >> bool hwrng_removed; >> + /* data transfer */ >> + struct completion have_data; >> + unsigned int data_avail; >> + /* minimal size returned by rng_buffer_size() */ >> +#if SMP_CACHE_BYTES < 32 >> + u8 data[32]; >> +#else >> + u8 data[SMP_CACHE_BYTES]; >> +#endif > > Let's move this logic to a macro in hw_random.h ? > >> }; >> >> static void random_recv_done(struct virtqueue *vq) >> @@ -39,14 +46,14 @@ static void random_recv_done(struct virtqueue *vq) >> } >> >> /* The host will fill any buffer we give it with sweet, sweet randomness. */ >> -static void register_buffer(struct virtrng_info *vi, u8 *buf, size_t size) >> +static void register_buffer(struct virtrng_info *vi) >> { >> struct scatterlist sg; >> >> - sg_init_one(&sg, buf, size); >> + sg_init_one(&sg, vi->data, sizeof(vi->data)); > > Note that add_early_randomness requests less: > size_t size = min_t(size_t, 16, rng_buffer_size()); > > maybe track how much was requested and grow up to sizeof(data)? I think this problem is managed by PATCH 3/4 as we reuse unused data of the buffer. > >> >> /* There should always be room for one buffer. */ >> - virtqueue_add_inbuf(vi->vq, &sg, 1, buf, GFP_KERNEL); >> + virtqueue_add_inbuf(vi->vq, &sg, 1, vi->data, GFP_KERNEL); > > > BTW no longer true if DMA API is in use ... not easy to fix, > I think some changes to virtio API to allow pre-mapping > s/g for DMA might be needed ... Is there something I can do here? Thanks, Laurent
Powered by blists - more mailing lists