Re: [patch] libstdc++/65142 Check read() result in std::random_device.

[Jonathan Wakely]

On 15/09/15 12:47 +0100, Jonathan Wakely wrote:

On 11/09/15 14:44 +0100, Jonathan Wakely wrote:

We should not silently ignore a failure to read from the random
device.

Tested powerpc64le-linux, committed to trunk. I'm going to commit this
to the gcc-5 branch too.

commit 2d2f7012dc3744dafef0de94dd845bd190253dbd
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Fri Feb 20 17:29:50 2015 +0000

  Check read() result in std::random_device.
        PR libstdc++/65142
        * src/c++11/random.cc (random_device::_M_getval()): Check read result.

diff --git a/libstdc++-v3/src/c++11/random.cc b/libstdc++-v3/src/c++11/random.cc
index edf900f..1d102c7 100644
--- a/libstdc++-v3/src/c++11/random.cc
+++ b/libstdc++-v3/src/c++11/random.cc
@@ -130,13 +130,17 @@ namespace std _GLIBCXX_VISIBILITY(default)
#endif

   result_type __ret;
+
#ifdef _GLIBCXX_HAVE_UNISTD_H
-    read(fileno(static_cast<FILE*>(_M_file)),
-        static_cast<void*>(&__ret), sizeof(result_type));
+    auto e = read(fileno(static_cast<FILE*>(_M_file)),
+                 static_cast<void*>(&__ret), sizeof(result_type));
#else
-    std::fread(static_cast<void*>(&__ret), sizeof(result_type),
-              1, static_cast<FILE*>(_M_file));
+    auto e = std::fread(static_cast<void*>(&__ret), sizeof(result_type),
+                       1, static_cast<FILE*>(_M_file));
#endif
+    if (e != sizeof(result_type))
+      __throw_runtime_error(__N("random_device could not read enough bytes"));
+
   return __ret;
 }

Florian pointed out that this code should handle short reads (or
EINTR) by retrying in a loop, so here's another attempt to fix it.

This also fixes the bug I introduced in the std::fread() case where it
expects e == sizeof(result_type) but fread will only return 0 or 1.

We could loop in the fread case too, but I'm not doing that. If
someone on non-POSIX targets cares enough they can make that change
later.

Any comments on this version?

Committed to trunk.

commit 6700c8c652da94332562fff465a1569d8fa9c94d
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Tue Sep 15 11:02:42 2015 +0100

   Fix handling of short reads in std::random_device

PR libstdc++/65142

        * src/c++11/random.cc (random_device::_M_getval()): Retry after short
        reads.

diff --git a/libstdc++-v3/src/c++11/random.cc b/libstdc++-v3/src/c++11/random.cc
index 1d102c7..f1d6125 100644
--- a/libstdc++-v3/src/c++11/random.cc
+++ b/libstdc++-v3/src/c++11/random.cc
@@ -130,16 +130,26 @@ namespace std _GLIBCXX_VISIBILITY(default)
#endif

    result_type __ret;
-
+    void* p = &__ret;
+    size_t n = sizeof(result_type);
#ifdef _GLIBCXX_HAVE_UNISTD_H
-    auto e = read(fileno(static_cast<FILE*>(_M_file)),
-                 static_cast<void*>(&__ret), sizeof(result_type));
+    do
+      {
+       const int e = read(fileno(static_cast<FILE*>(_M_file)), p, n);
+       if (e > 0)
+         {
+           n -= e;
+           p = static_cast<char*>(p) + e;
+         }
+       else if (e != -1 || errno != EINTR)
+         __throw_runtime_error(__N("random_device could not be read"));
+      }
+    while (n > 0);
#else
-    auto e = std::fread(static_cast<void*>(&__ret), sizeof(result_type),
-                       1, static_cast<FILE*>(_M_file));
+    const size_t e = std::fread(p, n, 1, static_cast<FILE*>(_M_file));
+    if (e != 1)
+      __throw_runtime_error(__N("random_device could not be read"));
#endif
-    if (e != sizeof(result_type))
-      __throw_runtime_error(__N("random_device could not read enough bytes"));

    return __ret;
  }