From fb03e57de20e48e201609f9f9594bc574462bccb Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 16 Mar 2016 14:19:08 -0700 Subject: [PATCH] buffer: backport --zero-fill-buffers cli 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. PR-URL: https://github.com/nodejs/node/pull/5745 Reviewed-By: Trevor Norris --- doc/api/buffer.markdown | 17 ++++++++++++ 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 ++++++++++++++++++++++ 6 files changed, 70 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 2a0b45d0e46a03..ca6e76623b938f 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/node.1 b/doc/node.1 index 0382a095db6d77..aeaf8911619028 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -95,6 +95,10 @@ Process v8 profiler output generated using the v8 option \fB\-\-prof\fR .BR \-\-v8\-options Print v8 command line options. +.TP +.BR \-\-zero\-fill\-buffers +Automatically zero-fills all newly allocated Buffer and SlowBuffer instances. + .TP .BR \-\-tls\-cipher\-list =\fIlist\fR Specify an alternative default TLS cipher list. (Requires Node.js to be built with crypto support. (Default)) diff --git a/src/node.cc b/src/node.cc index 2f08a1da378eaf..3ef56b2dd3726f 100644 --- a/src/node.cc +++ b/src/node.cc @@ -947,7 +947,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); @@ -3283,6 +3285,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" @@ -3422,6 +3426,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 83efaab210ea6f..d07632a101d491 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)); + } +}