Commit 870c1cd
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 2e612fe commit 870c1cd
4 files changed
Lines changed: 47 additions & 1 deletion
File tree
- doc/api
- test/ffi
- fixture_library
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -539,6 +539,8 @@ native memory directly. The caller must guarantee that: | |||
| 539 | 539 | * `length` stays within the allocated native region. | |
| 540 | 540 | * no native code frees or repurposes that memory while JavaScript still uses | |
| 541 | 541 | the `Buffer`. | |
| 542 | + * Memory protection is observed. For example, read-only memory pages must not | ||
| 543 | + be written to. | ||
| 542 | 544 | ||
| 543 | 545 | If these guarantees are not met, reading or writing the `Buffer` can corrupt | |
| 544 | 546 | memory or crash the process. | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -79,6 +79,10 @@ const fixtureSymbols = { | |||
| 79 | 79 | array_set_f64: { parameters: ['pointer', 'u64', 'f64'], result: 'void' }, | |
| 80 | 80 | }; | |
| 81 | 81 | ||
| 82 | + if (!common.isWindows) { | ||
| 83 | + fixtureSymbols.readonly_memory = { parameters: [], result: 'pointer' }; | ||
| 84 | + } | ||
| 85 | + | ||
| 82 | 86 | function cString(value) { | |
| 83 | 87 | return Buffer.from(`${value}\0`); | |
| 84 | 88 | } | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -2,10 +2,10 @@ | |||
| 2 | 2 | #include <stdint.h> | |
| 3 | 3 | #include <stdlib.h> | |
| 4 | 4 | #include <string.h> | |
| 5 | - | ||
| 6 | 5 | #ifdef _WIN32 | |
| 7 | 6 | #define FFI_EXPORT __declspec(dllexport) | |
| 8 | 7 | #else | |
| 8 | + #include <sys/mman.h> | ||
| 9 | 9 | #define FFI_EXPORT | |
| 10 | 10 | #endif | |
| 11 | 11 | ||
@@ -378,3 +378,12 @@ FFI_EXPORT void array_set_f64(double* arr, size_t index, double value) { | |||
| 378 | 378 | ||
| 379 | 379 | arr[index] = value; | |
| 380 | 380 | } | |
| 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 | ||
| Original file line number | Diff line number | Diff 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