Skip to content

Commit 3a53447

Browse files
authored
doc,test: mem protection must be observed in ffi
When using ffi.toBuffer, memory protection on any memory pages exposed must be observed by the caller, otherwise crashes will occur. Now documented, and tested. PR-URL: #62818 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent f5808ed commit 3a53447

4 files changed

Lines changed: 47 additions & 1 deletion

File tree

doc/api/ffi.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,8 @@ native memory directly. The caller must guarantee that:
539539
* `length` stays within the allocated native region.
540540
* no native code frees or repurposes that memory while JavaScript still uses
541541
the `Buffer`.
542+
* Memory protection is observed. For example, read-only memory pages must not
543+
be written to.
542544

543545
If these guarantees are not met, reading or writing the `Buffer` can corrupt
544546
memory or crash the process.

test/ffi/ffi-test-common.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ const fixtureSymbols = {
7979
array_set_f64: { parameters: ['pointer', 'u64', 'f64'], result: 'void' },
8080
};
8181

82+
if (!common.isWindows) {
83+
fixtureSymbols.readonly_memory = { parameters: [], result: 'pointer' };
84+
}
85+
8286
function cString(value) {
8387
return Buffer.from(`${value}\0`);
8488
}

test/ffi/fixture_library/ffi_test_library.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22
#include <stdint.h>
33
#include <stdlib.h>
44
#include <string.h>
5-
65
#ifdef _WIN32
76
#define FFI_EXPORT __declspec(dllexport)
87
#else
8+
#include <sys/mman.h>
99
#define FFI_EXPORT
1010
#endif
1111

@@ -378,3 +378,12 @@ FFI_EXPORT void array_set_f64(double* arr, size_t index, double value) {
378378

379379
arr[index] = value;
380380
}
381+
382+
#ifndef _WIN32
383+
FFI_EXPORT void* readonly_memory() {
384+
// TODO(bengl) Add a Windows version of this.
385+
386+
void* p = mmap(0, 4096, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
387+
return p;
388+
}
389+
#endif
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Flags: --experimental-ffi
2+
'use strict';
3+
const { skipIfFFIMissing, isWindows, skip } = require('../common');
4+
const assert = require('node:assert');
5+
const { spawnSync } = require('node:child_process');
6+
const { test } = require('node:test');
7+
const { fixtureSymbols, libraryPath } = require('./ffi-test-common');
8+
9+
skipIfFFIMissing();
10+
if (isWindows) {
11+
skip('This test currently relies on POSIX APIs');
12+
}
13+
14+
test('writing to readonly memory via buffer fails', () => {
15+
const symbols = JSON.stringify(fixtureSymbols);
16+
const libPath = JSON.stringify(libraryPath);
17+
const { stdout, status } = spawnSync(process.execPath, [
18+
'--experimental-ffi',
19+
'-p',
20+
`
21+
const ffi = require('node:ffi');
22+
const { functions } = ffi.dlopen(${libPath}, ${symbols})
23+
const p = functions.readonly_memory();
24+
const b = ffi.toBuffer(p, 4096, false);
25+
b[0] = 42;
26+
console.log('success');
27+
`,
28+
]);
29+
assert.notStrictEqual(status, 0);
30+
assert.strictEqual(stdout.length, 0);
31+
});

0 commit comments

Comments
 (0)