Date: Wed, 21 Oct 2009 06:21:29 +0400 From: Maxim Dounin <mdounin@mdounin.ru> To: Doug Barton <dougb@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Ruslan Ermilov <ru@FreeBSD.org> Subject: Re: svn commit: r198295 - in head/sys: kern sys Message-ID: <20091021022128.GR1144@mdounin.ru> In-Reply-To: <4ADDFCCC.8070204__19944.4177946591$1256062254$gmane$org@FreeBSD.org> References: <200910201636.n9KGapWo072133@svn.freebsd.org> <4ADDFCCC.8070204__19944.4177946591$1256062254$gmane$org@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hello! On Tue, Oct 20, 2009 at 11:09:16AM -0700, Doug Barton wrote: > Was this patch publicly reviewed anywhere? If so and I missed it I > apologize. Not really. It was part of suggested patch during secteam@ work on FreeBSD-SA-08:11.arc4random, but was dropped as irrelevant to advisory itself. > Otherwise it would be very nice if it got some discussion, > review, etc. before it proceeds any further. > > Something as important as how we use random numbers, especially in the > kernel, and especially as the system is initializing and entropy is > low really needs a lot of review. This is not meant as an aspersion on > Ruslan, Mr. Dounin, or anyone else involved. Sure. Feel free to ask if you have questions. But actually the bad thing is that this patch just can't break anything as there is nothing to break. Without the patch kernel has no entropy in random(9) until after proc0_post(), and no entropy in arc4random(9) until after reseed from userland. Maxim Dounin > > > Doug > > > Ruslan Ermilov wrote: > > Author: ru > > Date: Tue Oct 20 16:36:51 2009 > > New Revision: 198295 > > URL: http://svn.freebsd.org/changeset/base/198295 > > > > Log: > > Random number generator initialization cleanup: > > > > - Introduce new SI_SUB_RANDOM point in boot sequence to make it > > clear from where one may start using random(9). It should be as > > early as possible, so place it just after SI_SUB_CPU where we > > have some randomness on most platforms via get_cyclecount(). > > > > - Move stack protector initialization to be after SI_SUB_RANDOM > > as before this point we have no randomness at all. This fixes > > stack protector to actually protect stack with some random guard > > value instead of a well-known one. > > > > Note that this patch doesn't try to address arc4random(9) issues. > > With current code, it will be implicitly seeded by stack protector > > and hence will get the same entropy as random(9). It will be > > securely reseeded once /dev/random is feeded by some entropy from > > userland. > > > > Submitted by: Maxim Dounin <mdounin@mdounin.ru> > > MFC after: 3 days > > > > Modified: > > head/sys/kern/init_main.c > > head/sys/kern/stack_protector.c > > head/sys/sys/kernel.h > > > > Modified: head/sys/kern/init_main.c > > ============================================================================== > > --- head/sys/kern/init_main.c Tue Oct 20 16:32:38 2009 (r198294) > > +++ head/sys/kern/init_main.c Tue Oct 20 16:36:51 2009 (r198295) > > @@ -570,6 +570,19 @@ proc0_post(void *dummy __unused) > > } > > SYSINIT(p0post, SI_SUB_INTRINSIC_POST, SI_ORDER_FIRST, proc0_post, NULL); > > > > +static void > > +random_init(void *dummy __unused) > > +{ > > + > > + /* > > + * After CPU has been started we have some randomness on most > > + * platforms via get_cyclecount(). For platforms that don't > > + * we will reseed random(9) in proc0_post() as well. > > + */ > > + srandom(get_cyclecount()); > > +} > > +SYSINIT(random, SI_SUB_RANDOM, SI_ORDER_FIRST, random_init, NULL); > > + > > /* > > *************************************************************************** > > **** > > > > Modified: head/sys/kern/stack_protector.c > > ============================================================================== > > --- head/sys/kern/stack_protector.c Tue Oct 20 16:32:38 2009 (r198294) > > +++ head/sys/kern/stack_protector.c Tue Oct 20 16:36:51 2009 (r198295) > > @@ -28,5 +28,4 @@ __stack_chk_init(void *dummy __unused) > > for (i = 0; i < __arraycount(guard); i++) > > __stack_chk_guard[i] = guard[i]; > > } > > -/* SI_SUB_EVENTHANDLER is right after SI_SUB_LOCK used by arc4rand() init. */ > > -SYSINIT(stack_chk, SI_SUB_EVENTHANDLER, SI_ORDER_ANY, __stack_chk_init, NULL); > > +SYSINIT(stack_chk, SI_SUB_RANDOM, SI_ORDER_ANY, __stack_chk_init, NULL); > > > > Modified: head/sys/sys/kernel.h > > ============================================================================== > > --- head/sys/sys/kernel.h Tue Oct 20 16:32:38 2009 (r198294) > > +++ head/sys/sys/kernel.h Tue Oct 20 16:36:51 2009 (r198295) > > @@ -109,6 +109,7 @@ enum sysinit_sub_id { > > SI_SUB_VNET_PRELINK = 0x1E00000, /* vnet init before modules */ > > SI_SUB_KLD = 0x2000000, /* KLD and module setup */ > > SI_SUB_CPU = 0x2100000, /* CPU resource(s)*/ > > + SI_SUB_RANDOM = 0x2120000, /* random number generator */ > > SI_SUB_KDTRACE = 0x2140000, /* Kernel dtrace hooks */ > > SI_SUB_MAC = 0x2180000, /* TrustedBSD MAC subsystem */ > > SI_SUB_MAC_POLICY = 0x21C0000, /* TrustedBSD MAC policies */ > > > > > -- > > Improve the effectiveness of your Internet presence with > a domain name makeover! http://SupersetSolutions.com/ > > _______________________________________________ > svn-src-all@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/svn-src-all > To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20091021022128.GR1144>