From c64d0b0fccfe4788354542c45b8ee11f46c77ea3 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 16 Mar 2016 13:42:27 -0700 Subject: [PATCH] buffer: backport --zero-fill-buffers command line option This backports the --zero-fill-buffers command line flag introduced in master. When used, all Buffer and SlowBuffer instances will zero fill by default. This does *not* backport any of the other Buffer API or behavior changes. --- doc/api/buffer.markdown | 17 ++++++++++++ doc/api/cli.markdown | 3 +++ doc/node.1 | 4 +++ src/node.cc | 8 +++++- src/node_buffer.cc | 11 ++++++-- src/node_buffer.h | 3 +++ test/parallel/test-buffer-zero-fill-cli.js | 30 ++++++++++++++++++++++ 7 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-buffer-zero-fill-cli.js diff --git a/doc/api/buffer.markdown b/doc/api/buffer.markdown index 4c8a48de906508..a40921f780996e 100644 --- a/doc/api/buffer.markdown +++ b/doc/api/buffer.markdown @@ -145,6 +145,23 @@ for (var b of buf) Additionally, the [`buf.values()`][], [`buf.keys()`][], and [`buf.entries()`][] methods can be used to create iterators. +## The `--zero-fill-buffers` command line option + +Node.js can be started using the `--zero-fill-buffers` command line option to +force all newly allocated `Buffer` and `SlowBuffer` instances created using +either `new Buffer(size)` and `new SlowBuffer(size)` to be *automatically +zero-filled* upon creation. Use of this flag *changes the default behavior* of +these methods and *can have a significant impact* on performance. Use of the +`--zero-fill-buffers` option is recommended only when absolutely necessary to +enforce that newly allocated `Buffer` instances cannot contain potentially +sensitive data. + +``` +$ node --zero-fill-buffers +> Buffer(5); + +``` + ## Class: Buffer The Buffer class is a global type for dealing with binary data directly. diff --git a/doc/api/cli.markdown b/doc/api/cli.markdown index 2910c782d2ad8e..13e8a396e40c2a 100644 --- a/doc/api/cli.markdown +++ b/doc/api/cli.markdown @@ -93,6 +93,9 @@ instances. Track heap object allocations for heap snapshots. +### `--zero-fill-buffers` + +Automatically zero-fills all newly allocated Buffer and SlowBuffer instances. ### `--prof-process` diff --git a/doc/node.1 b/doc/node.1 index 661c0d56192f75..e1b2f77dd9b307 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -87,6 +87,10 @@ of the event loop. .BR \-\-track\-heap-objects Track heap object allocations for heap snapshots. +.TP +.BR \-\-zero\-fill\-buffers +Automatically zero-fills all newly allocated Buffer and SlowBuffer instances. + .TP .BR \-\-prof\-process Process v8 profiler output generated using the v8 option \fB\-\-prof\fR diff --git a/src/node.cc b/src/node.cc index 5da25f8407f265..dc648a4a5452c3 100644 --- a/src/node.cc +++ b/src/node.cc @@ -946,7 +946,9 @@ Local WinapiErrnoException(Isolate* isolate, void* ArrayBufferAllocator::Allocate(size_t size) { - if (env_ == nullptr || !env_->array_buffer_allocator_info()->no_zero_fill()) + if (env_ == nullptr || + !env_->array_buffer_allocator_info()->no_zero_fill() || + zero_fill_all_buffers) return calloc(size, 1); env_->array_buffer_allocator_info()->reset_fill_flag(); return malloc(size); @@ -3261,6 +3263,8 @@ static void PrintHelp() { "snapshots\n" " --prof-process process v8 profiler output generated\n" " using --prof\n" + " --zero-fill-buffers automatically zero-fill all newly allocated\n" + " Buffer and SlowBuffer instances\n" " --v8-options print v8 command line options\n" #if HAVE_OPENSSL " --tls-cipher-list=val use an alternative default TLS cipher list\n" @@ -3400,6 +3404,8 @@ static void ParseArgs(int* argc, } else if (strcmp(arg, "--prof-process") == 0) { prof_process = true; short_circuit = true; + } else if (strcmp(arg, "--zero-fill-buffers") == 0) { + zero_fill_all_buffers = true; } else if (strcmp(arg, "--v8-options") == 0) { new_v8_argv[new_v8_argc] = "--help"; new_v8_argc += 1; diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 1c60a311a531e8..bee82c5ab76658 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -48,7 +48,14 @@ CHECK_NOT_OOB(end <= end_max); \ size_t length = end - start; +#define BUFFER_MALLOC(length) \ + zero_fill_all_buffers ? calloc(length, 1) : malloc(length) + namespace node { + +// if true, all Buffer and SlowBuffer instances will automatically zero-fill +bool zero_fill_all_buffers = false; + namespace Buffer { using v8::ArrayBuffer; @@ -220,7 +227,7 @@ MaybeLocal New(Isolate* isolate, // nullptr for zero-sized allocation requests. Normalize by always using // a nullptr. if (length > 0) { - data = static_cast(malloc(length)); + data = static_cast(BUFFER_MALLOC(length)); if (data == nullptr) return Local(); @@ -266,7 +273,7 @@ MaybeLocal New(Environment* env, size_t length) { void* data; if (length > 0) { - data = malloc(length); + data = BUFFER_MALLOC(length); if (data == nullptr) return Local(); } else { diff --git a/src/node_buffer.h b/src/node_buffer.h index 503cbb167547a5..686450d984e6f9 100644 --- a/src/node_buffer.h +++ b/src/node_buffer.h @@ -5,6 +5,9 @@ #include "v8.h" namespace node { + +extern bool zero_fill_all_buffers; + namespace Buffer { static const unsigned int kMaxLength = diff --git a/test/parallel/test-buffer-zero-fill-cli.js b/test/parallel/test-buffer-zero-fill-cli.js new file mode 100644 index 00000000000000..2f6c011c5c4ff0 --- /dev/null +++ b/test/parallel/test-buffer-zero-fill-cli.js @@ -0,0 +1,30 @@ +'use strict'; +// Flags: --zero-fill-buffers + +// when using --zero-fill-buffers, every Buffer and SlowBuffer +// instance must be zero filled upon creation + +require('../common'); +const SlowBuffer = require('buffer').SlowBuffer; +const assert = require('assert'); + +function isZeroFilled(buf) { + for (let n = 0; n < buf.length; n++) + if (buf[n] > 0) return false; + return true; +} + +// This can be somewhat unreliable because the +// allocated memory might just already happen to +// contain all zeroes. The test is run multiple +// times to improve the reliability. +for (let i = 0; i < 50; i++) { + const bufs = [ + SlowBuffer(20), + Buffer(20), + new SlowBuffer(20) + ]; + for (const buf of bufs) { + assert(isZeroFilled(buf)); + } +}