superreview granted: [Bug 182758] freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom : [Attachment 236342] update

Wan-Teh Chang <wtchang@redhat.com> has granted Julien Pierre
<julien.pierre.bugs@sun.com>'s request for superreview:
Bug 182758: freebl PRNG hashes netstat and /dev/urandom data rather than just
using /dev/urandom
https://bugzilla.mozilla.org/show_bug.cgi?id=182758

Attachment 236342: update
https://bugzilla.mozilla.org/attachment.cgi?id=236342&action=edit

------- Additional Comments from Wan-Teh Chang <wtchang@redhat.com>
r=wtc.

Please remove the 4 extraneous semicolons after closing curly
braces.  Just search for "};" in the file and remove the ";".

The following are optional suggested changes and comments.
You can ignore them, especially 7, 8, and 9.

1. max_entropy_buf_len should be renamed entropy_buf_len.
The length of entropy_buf is fixed, so "max" doesn't make
sense.

2. In this comment:

>+/* Buffer entropy data, and feed it to the RNG, 4 KB at a time.
>+ * Returns error if RNG refuses data.

Change "4 KB" to "max_entropy_buf_len bytes".

Change "RNG refuses data" to "RNG_RandomUpdate fails".

3. Since entropy_collected has been renamed entropy_buffered,
perhaps CollectEntropy should also be renamed BufferEntropy?

4. In CollectEntropy, the local variable 'processed' is not
necessary.  You can update *total_fed directly, i.e., replace

	processed += tocopy;

by

	*total_fed += tocopy;

at the end of the while loop.

5. In RNG_kstat, you have:

>+	      if (-1 == kstat_read(kc, ksp, NULL)) {

and

>+    if (kstat_close(kc)) {

It would look nicer to use the same method to test for the failure
of kstat_read and kstat_close.

6. The local variable 'total_fed' in RNG_kstat is not necessary.
RNG_kstat can actually pass 'fed' through to the two CollectEntropy
calls (instead of &total_fed).

7

>+    if (!bytes) {
>+	  /* On Solaris 8, /dev/urandom isn't available, so we use libkstat. */

>+	  PRUint32 kstat_bytes = 0;
>+	  if (SECSuccess != RNG_kstat(&kstat_bytes)) {
>+	      PORT_Assert(0);
>+	  };
>+	  bytes += kstat_bytes;
>+	  PORT_Assert(bytes);
>+    }

Since 'bytes' is 0, you can just pass &bytes instead of
&kstat_bytes to RNG_kstat.  Since the type of 'bytes' is
size_t, this change would require propagating that type
to the 'fed' parameter of RNG_kstat and possibly the
'total_fed' parameter of CollectEntropy.

8. RNG_kstat can call RNG_RandomUpdate directly.  It doesn't need to
buffer the entropy in CollectEntropy.  Another way to say this is that
if the CollectEntropy function is a good idea, then we should use it
throughout the RNG_SystemInfoForRNG function.

For example, the while loop that steps through all the environment
variables in the 'environ' array is very similar to your RNG_kstat
function.  It should use the same method (either buffered or
non-buffered) as RNG_kstat to feed its data to RNG_RandomUpdate.

9. RNG_SystemInfoForRNG already has a buffer 'buf'.  Ideally you should
be able to use that buffer instead of 'entropy_buf' in RNG_kstat.
0
bugzilla
9/1/2006 4:55:04 PM
mozilla.dev.super-review 29307 articles. 3 followers. Post Follow

0 Replies
452 Views

Similar Articles

[PageSpeed] 1

Reply:

Similar Artilces:

superreview requested: [Bug 182758] freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom : [Attachment 236342] update
Julien Pierre <julien.pierre.bugs@sun.com> has asked Wan-Teh Chang <wtchang@redhat.com> for superreview: Bug 182758: freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom https://bugzilla.mozilla.org/show_bug.cgi?id=182758 Attachment 236342: update https://bugzilla.mozilla.org/attachment.cgi?id=236342&action=edit ------- Additional Comments from Julien Pierre <julien.pierre.bugs@sun.com> This patch contains several changes : 1) More comments 2) CollectEntropy and RNG_kstat are both changed to return a SECStatus . This is to ea...

superreview requested: [Bug 182758] freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom : [Attachment 235522] On Solaris, use only /dev/urandom if it is available. If
Julien Pierre <julien.pierre.bugs@sun.com> has asked Wan-Teh Chang <wtchang@redhat.com> for superreview: Bug 182758: freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom https://bugzilla.mozilla.org/show_bug.cgi?id=182758 Attachment 235522: On Solaris, use only /dev/urandom if it is available. If not, use libkstat https://bugzilla.mozilla.org/attachment.cgi?id=235522&action=edit ------- Additional Comments from Julien Pierre <julien.pierre.bugs@sun.com> In the libkstat case, I am feeding all kernel statistics to the PRNG, 4 KB ...

superreview denied: [Bug 182758] freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom : [Attachment 235522] On Solaris, use only /dev/urandom if it is available. If not
Wan-Teh Chang <wtchang@redhat.com> has denied Julien Pierre <julien.pierre.bugs@sun.com>'s request for superreview: Bug 182758: freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom https://bugzilla.mozilla.org/show_bug.cgi?id=182758 Attachment 235522: On Solaris, use only /dev/urandom if it is available. If not, use libkstat https://bugzilla.mozilla.org/attachment.cgi?id=235522&action=edit ------- Additional Comments from Wan-Teh Chang <wtchang@redhat.com> These are just some minor problems. But since there are many, I wa...

superreview requested: [Bug 182758] freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom : [Attachment 235586] Update
Julien Pierre <julien.pierre.bugs@sun.com> has asked Nelson Bolyard <nelson@bolyard.com> for superreview: Bug 182758: freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom https://bugzilla.mozilla.org/show_bug.cgi?id=182758 Attachment 235586: Update https://bugzilla.mozilla.org/attachment.cgi?id=235586&action=edit ------- Additional Comments from Julien Pierre <julien.pierre.bugs@sun.com> Wan-Teh, This was written from scratch, it wasn't sample code. I switched from assert to PORT_Assert, as well as from malloc/free to ...

superreview cancelled: [Bug 182758] freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom : [Attachment 235586] Update
Nelson Bolyard <nelson@bolyard.com> has cancelled Julien Pierre <julien.pierre.bugs@sun.com>'s request for superreview: Bug 182758: freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom https://bugzilla.mozilla.org/show_bug.cgi?id=182758 Attachment 235586: Update https://bugzilla.mozilla.org/attachment.cgi?id=235586&action=edit ------- Additional Comments from Nelson Bolyard <nelson@bolyard.com> I have some minor quibbles with this patch. 1) rather than seeing all the new code be "ifdef solaris" and "ifndef ...

superreview cancelled: [Bug 182758] freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom : [Attachment 236154] Update with feedback from Nelson and Wan-Teh
Julien Pierre <julien.pierre.bugs@sun.com> has cancelled Julien Pierre <julien.pierre.bugs@sun.com>'s request for superreview: Bug 182758: freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom https://bugzilla.mozilla.org/show_bug.cgi?id=182758 Attachment 236154: Update with feedback from Nelson and Wan-Teh https://bugzilla.mozilla.org/attachment.cgi?id=236154&action=edit ------- Additional Comments from Julien Pierre <julien.pierre.bugs@sun.com> This patch contains several changes : 1) More comments 2) CollectEntropy and R...

superreview requested: [Bug 182758] freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom : [Attachment 236154] Update with feedback from Nelson and Wan-Teh
Julien Pierre <julien.pierre.bugs@sun.com> has asked Wan-Teh Chang <wtchang@redhat.com> for superreview: Bug 182758: freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom https://bugzilla.mozilla.org/show_bug.cgi?id=182758 Attachment 236154: Update with feedback from Nelson and Wan-Teh https://bugzilla.mozilla.org/attachment.cgi?id=236154&action=edit ------- Additional Comments from Julien Pierre <julien.pierre.bugs@sun.com> - remove kprintf statements - rename max_entropy_len to max_entropy_buf_len - initialize buffered to zer...

superreview granted: [Bug 128673] jprof should use Linux's /dev/rtc for up-to-8KHz sampling : [Attachment 134273] slightly improved patch
David Baron <dbaron@dbaron.org> has granted Brian Ryner <bryner@brianryner.com>'s request for superreview: Bug 128673: jprof should use Linux's /dev/rtc for up-to-8KHz sampling http://bugzilla.mozilla.org/show_bug.cgi?id=128673 Attachment 134273: slightly improved patch http://bugzilla.mozilla.org/attachment.cgi?id=134273&action=edit ------- Additional Comments from David Baron <dbaron@dbaron.org> >+ if (!IS_POWER_OF_TWO(rtcHz) || rtcHz < 2) { >+ fprintf(stderr, "JP_RTC_HZ must be power of two and > 2, " Slight dis...

superreview granted: [Bug 422848] cycle collector warnings about insufficient traverse/unlink should print path to expected garbage : [Attachment 309311] interdiff -w /dev/null <attachment 309310>
Peter Van der Beken <peterv@propagandism.org> has granted David Baron [:dbaron] (reduced activity until April 1) <dbaron@mozilla.com>'s request for superreview: Bug 422848: cycle collector warnings about insufficient traverse/unlink should print path to expected garbage https://bugzilla.mozilla.org/show_bug.cgi?id=422848 Attachment 309311: interdiff -w /dev/null <attachment 309310> https://bugzilla.mozilla.org/attachment.cgi?id=309311&action=edit ...

Merging dev-gaia and dev-b2g into dev-fxos
--001a113ce93ebce35d051e4c0c73 Content-Type: text/plain; charset=UTF-8 Hello people of Firefox OS, After a discussion we have decided that the distinction between dev-gaia and dev-b2g mailing lists is not enough to warrant maintaining two lists. So we are deprecating both in favor of dev-fxos. So if you are subscribed to one of the aforementioned lists, you will be subscribed to the new dev-fxos list and we will shortly be decommissioning dev-gaia and dev-b2g. Thanks! Michael --001a113ce93ebce35d051e4c0c73 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: qu...

Merging dev-gaia and dev-b2g into dev-fxos
--001a113ce93ebce35d051e4c0c73 Content-Type: text/plain; charset=UTF-8 Hello people of Firefox OS, After a discussion we have decided that the distinction between dev-gaia and dev-b2g mailing lists is not enough to warrant maintaining two lists. So we are deprecating both in favor of dev-fxos. So if you are subscribed to one of the aforementioned lists, you will be subscribed to the new dev-fxos list and we will shortly be decommissioning dev-gaia and dev-b2g. Thanks! Michael --001a113ce93ebce35d051e4c0c73 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: qu...

superreview granted: [Bug 326076] Use menulist for server secure connection (instead of radiogroup) : [Attachment 314075] Updated for review comments
neil@parkwaycc.co.uk <neil@httl.net> has granted Giacomo Magnini <prometeo.bugs@gmail.com>'s request for superreview: Bug 326076: Use menulist for server secure connection (instead of radiogroup) https://bugzilla.mozilla.org/show_bug.cgi?id=326076 Attachment 314075: Updated for review comments https://bugzilla.mozilla.org/attachment.cgi?id=314075&action=edit ...

superreview granted: [Bug 249903] nsGNOMERegistry::HandlerExists uses gconf client after g_object_unref()ing it : [Attachment 152362] updated patch as requested by the reviewer
Darin Fisher (IBM) <darin@meer.net> has granted Christian Persch <chpe@gnome.org>'s request for superreview: Bug 249903: nsGNOMERegistry::HandlerExists uses gconf client after g_object_unref()ing it http://bugzilla.mozilla.org/show_bug.cgi?id=249903 Attachment 152362: updated patch as requested by the reviewer http://bugzilla.mozilla.org/attachment.cgi?id=152362&action=edit ------- Additional Comments from Darin Fisher (IBM) <darin@meer.net> >Index: nsGNOMERegistry.cpp >+ return isEnabled ? PR_TRUE : PR_FALSE; return isEnabled != FALSE; ...

superreview granted: [Bug 227986] EmbedPrompter should use GtkComboBox on gtk+ 2.4 : [Attachment 137398] updated patch with the changes requested by the reviewer
Christopher Blizzard <blizzard@mozilla.org> has granted Christopher Blizzard <blizzard@mozilla.org>'s request for superreview: Bug 227986: EmbedPrompter should use GtkComboBox on gtk+ 2.4 http://bugzilla.mozilla.org/show_bug.cgi?id=227986 Attachment 137398: updated patch with the changes requested by the reviewer http://bugzilla.mozilla.org/attachment.cgi?id=137398&action=edit ...

Web resources about - superreview granted: [Bug 182758] freebl PRNG hashes netstat and /dev/urandom data rather than just using /dev/urandom : [Attachment 236342] update - mozilla.dev.super-review

Resources last updated: 11/26/2015 7:08:11 AM