From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jens Axboe Subject: Re: virtio-rng only returns zeros with CONFIG_HW_RANDOM=m Date: Thu, 30 May 2013 09:18:40 +0200 Message-ID: <20130530071840.GR22724@kernel.dk> References: <20130224231154.GA30477@hall.aurel32.net> <8738wifwb2.fsf@rustcorp.com.au> <20130227132213.GF4615@kernel.dk> <874ndlmg0h.fsf@rustcorp.com.au> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Aurelien Jarno , kvm@vger.kernel.org, Ian Molton , Matt Mackall , Herbert Xu , Ben Hutchings To: Rusty Russell Return-path: Received: from merlin.infradead.org ([205.233.59.134]:40310 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S967578Ab3E3HTb (ORCPT ); Thu, 30 May 2013 03:19:31 -0400 Content-Disposition: inline In-Reply-To: <874ndlmg0h.fsf@rustcorp.com.au> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, May 30 2013, Rusty Russell wrote: > Jens Axboe writes: > > On Wed, Feb 27 2013, Rusty Russell wrote: > >> Aurelien Jarno writes: > >> > Hi, > >> > > >> > I have noticed that virtio-rng only returns zero for kernels >= 2.6.33 > >> > built with CONFIG_HW_RANDOM=m. This is a bit much too predictable for a > >> > random generator ;-). > >> > >> Wow. Fortunately, all of SLES, RHEL, Ubuntu or Fedora set > >> CONFIG_HW_RANDOM=y. What do they know that we don't? > >> > >> Oops, looks like Debian testing: config-3.2.0-4-amd64:CONFIG_HW_RANDOM=m > >> > >> > The reason for that is virtio expects guest real addresses, while > >> > rng_core.ko (ie when built as a module) is passing a vmalloced buffer > >> > to the virtio-rng read function, declared as such: > >> > > >> > static u8 rng_buffer[SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES] > >> > __cacheline_aligned; > >> > >> Yuck... It would be nice if this has oopsed. Jens, what about this patch? > >> > >> Cheers, > >> Rusty. > >> > >> Subject: scatterlist: sg_set_buf() argument must be in linear mapping. > >> > >> Signed-off-by: Rusty Russell > >> > >> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h > >> index 4bd6c06..9365375 100644 > >> --- a/include/linux/scatterlist.h > >> +++ b/include/linux/scatterlist.h > >> @@ -111,6 +111,9 @@ static inline struct page *sg_page(struct scatterlist *sg) > >> static inline void sg_set_buf(struct scatterlist *sg, const void *buf, > >> unsigned int buflen) > >> { > >> +#ifdef CONFIG_DEBUG_SG > >> + BUG_ON(!virt_addr_valid(buf)); > >> +#endif > >> sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf)); > >> } > > > > Looks good to me, in lieu of being able to return an error. Want me to > > queue it up? > > > > -- > > Jens Axboe > > Ping? I haven't seen this go into Linus' tree... I forget if I didn't get a reply, or whether it just got lost! Queued up now, at least. -- Jens Axboe