Commit 5124e464 authored by Findit's avatar Findit

Revert "[base] Use getrandom on Linux."

This reverts commit 68d3758b.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 801909 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzY4ZDM3NThiMDEwZGMzZGIxN2NkNWU1MzdmMDAxNGE0N2Y0MDI5MDYM

Sample Failed Build: https://ci.chromium.org/b/8870873385800887712

Sample Failed Step: compile

Original change's description:
> [base] Use getrandom on Linux.
> 
> Fall back to reading from urandom only when that fails.
> 
> Bug: 995996
> Change-Id: Id503ca2dc412a392d5a7e252fcfff65b61d0c20e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1762604
> Commit-Queue: Chris Palmer <palmer@chromium.org>
> Reviewed-by: Julien Tinnes <jln@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#801909}


Change-Id: If682cc741bd401fc6ab2a3e9e8173ac8e20d6e75
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 995996
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2377418
Cr-Commit-Position: refs/heads/master@{#801937}
parent ef4aa667
......@@ -16,34 +16,32 @@
#include "base/posix/eintr_wrapper.h"
#include "build/build_config.h"
#if defined(OS_LINUX) || defined(OS_CHROMEOS)
#include "third_party/lss/linux_syscall_support.h"
#endif
#if !defined(OS_IOS) && !defined(OS_NACL)
#if defined(OS_MAC)
// TODO(crbug.com/995996): Waiting for this header to appear in the iOS SDK.
// (See below.)
// (See below.) We'll also use this on other POSIX platforms in the future (and
// change the #if condition then).
#include <sys/random.h>
#endif
namespace {
#if defined(OS_AIX)
// AIX has no 64-bit support for O_CLOEXEC.
static constexpr int kOpenFlags = O_RDONLY;
#else
static constexpr int kOpenFlags = O_RDONLY | O_CLOEXEC;
#endif
// We keep the file descriptor for /dev/urandom around so we don't need to
// reopen it (which is expensive), and since we may not even be able to reopen
// it if we are later put in a sandbox. This class wraps the file descriptor so
// we can use a static-local variable to handle opening it on the first access.
class URandomFd {
public:
URandomFd() : fd_(HANDLE_EINTR(open("/dev/urandom", kOpenFlags))) {
CHECK(fd_ >= 0) << "Cannot open /dev/urandom";
#if defined(OS_AIX)
// AIX has no 64-bit support for open falgs such as -
// O_CLOEXEC, O_NOFOLLOW and O_TTY_INIT
URandomFd() : fd_(HANDLE_EINTR(open("/dev/urandom", O_RDONLY))) {
DPCHECK(fd_ >= 0) << "Cannot open /dev/urandom";
}
#else
URandomFd() : fd_(HANDLE_EINTR(open("/dev/urandom", O_RDONLY | O_CLOEXEC))) {
DPCHECK(fd_ >= 0) << "Cannot open /dev/urandom";
}
#endif
~URandomFd() { close(fd_); }
......@@ -57,24 +55,8 @@ class URandomFd {
namespace base {
// NOTE: In an ideal future, all implementations of this function will just
// wrap BoringSSL's `RAND_bytes`. TODO(crbug.com/995996): Figure out the
// build/test/performance issues with dcheng's CL
// (https://chromium-review.googlesource.com/c/chromium/src/+/1545096) and land
// it or some form of it.
void RandBytes(void* output, size_t output_length) {
#if (defined(OS_LINUX) || defined(OS_CHROMEOS)) && !defined(OS_NACL)
// We have to call `getrandom` via Linux Syscall Support, rather than through
// the libc wrapper, because we might not have an up-to-date libc (e.g. on
// some bots).
const ssize_t r = HANDLE_EINTR(sys_getrandom(output, output_length, 0));
// Return success only on total success. In case errno == ENOSYS (or any other
// error), we'll fall through to reading from urandom below.
if (output_length == static_cast<size_t>(r)) {
return;
}
#elif defined(OS_MAC)
#if defined(OS_MAC)
// TODO(crbug.com/995996): Enable this on iOS too, when sys/random.h arrives
// in its SDK.
if (__builtin_available(macOS 10.12, *)) {
......@@ -82,13 +64,9 @@ void RandBytes(void* output, size_t output_length) {
return;
}
}
// Fall through to reading from urandom on < 10.12:
#endif
// If the OS-specific mechanisms didn't work, fall through to reading from
// urandom.
//
// TODO(crbug.com/995996): When we no longer need to support old Linux
// kernels, we can get rid of this /dev/urandom branch altogether.
const int urandom_fd = GetUrandomFD();
const bool success =
ReadFromFD(urandom_fd, static_cast<char*>(output), output_length);
......
......@@ -22,7 +22,6 @@
#include "base/files/scoped_file.h"
#include "base/logging.h"
#include "base/posix/eintr_wrapper.h"
#include "base/rand_util.h"
#include "build/build_config.h"
#include "components/nacl/common/nacl_switches.h"
#include "components/nacl/loader/nonsfi/nonsfi_sandbox.h"
......@@ -128,11 +127,6 @@ void NaClSandbox::InitializeLayerOneSandbox() {
// Check that IsSandboxed() works. We should not be sandboxed at this point.
CHECK(!IsSandboxed()) << "Unexpectedly sandboxed!";
// Open /dev/urandom while we can. This enables `base::RandBytes` to work. We
// don't need to store the resulting file descriptor; it's a singleton and
// subsequent calls to `GetUrandomFD` will return it.
CHECK_GE(base::GetUrandomFD(), 0);
if (setuid_sandbox_client_->IsSuidSandboxChild()) {
setuid_sandbox_client_->CloseDummyFile();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment