Skip to content

Commit 4d5b059

Browse files
committed
fix: reject promises in the arguments to syscall.callNow()
Vats which hold device nodes (`d-NN` references) can use `syscall.callNow()` on them, to make a synchronous invocation (which can return data). The outbound arguments and return data are capdata, which is translated through c-lists just like regular `syscall.send()` and promise resolution. However devices do not (currently) handle Promises at all. The kernel-to-device c-list translation will panic the kernel if asked to translate a promise reference (`kpNN`). Vats should not be able to panic the kernel, even if we give them access to a device node. This changes the vat-to-kernel translator to reject promise references in the arguments of `callNow`, making it a vat-fatal error. This will terminate the vat, but leave the kernel running. In the long run (#1346), devices should accept Promises, but it will take more work (and probably require devices to operate on a much lower level than vats do). But this should fix the immediate kernel panic. Note that killing a vat is not exactly friendly either. The bug described in issue #1358 was triggered by user REPL input causing the HTTP vat to try sending a Promise into the Command device, killing the kernel. With this change, this will instead kill the HTTP vat, which breaks the REPL, rendering the system mostly unusable. But at least the attribution is correct. We have another fix in the works that will change liveslots.js to catch this situation during the call to `D(devnode).methname(args)`, which should reduce the blast radius to merely throw an exception in `D()`, rather than killing the whole vat. refs #1358
1 parent 3a45930 commit 4d5b059

File tree

3 files changed

+35
-0
lines changed

3 files changed

+35
-0
lines changed

packages/SwingSet/src/kernel/vatTranslator.js

+6
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,12 @@ function makeTranslateVatSyscallToKernelSyscall(vatID, kernelKeeper) {
149149
if (type !== 'device') {
150150
throw new Error(`doCallNow must target a device, not ${dev}`);
151151
}
152+
for (const slot of args.slots) {
153+
assert(
154+
parseVatSlot(slot).type !== 'promise',
155+
`syscall.callNow() args cannot include promises like ${slot}`,
156+
);
157+
}
152158
const kernelSlots = args.slots.map(slot => mapVatSlotToKernelSlot(slot));
153159
const kernelData = harden({ ...args, slots: kernelSlots });
154160
// prettier-ignore

packages/SwingSet/test/files-devices/bootstrap-2.js

+12
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,18 @@ export function buildRootObject(vatPowers, vatParameters) {
8484
},
8585
});
8686
D(devices.command).registerInboundHandler(handler);
87+
} else if (argv[0] === 'promise1') {
88+
const p = Promise.resolve();
89+
log('sending Promise');
90+
try {
91+
// this will be rejected immediately, killing the vat shortly
92+
D(devices.d0).send({ p });
93+
// shouldn't get here
94+
log('oops: survived sending Promise');
95+
} catch (e) {
96+
// we aren't currently killed until the end of the crank
97+
log('good: callNow failed');
98+
}
8799
} else {
88100
throw new Error(`unknown argv mode '${argv[0]}'`);
89101
}

packages/SwingSet/test/test-devices.js

+17
Original file line numberDiff line numberDiff line change
@@ -419,3 +419,20 @@ test.serial('command deliver', async t => {
419419
t.deepEqual(c.dump().log, ['handle-0-missing', 'handle-1-errory']);
420420
t.deepEqual(rejection, { response: 'body' });
421421
});
422+
423+
test('callNow refuses promises', async t => {
424+
const config = {
425+
bootstrap: 'bootstrap',
426+
vats: {
427+
bootstrap: {
428+
bundle: t.context.data.bootstrap2,
429+
creationOptions: { enableSetup: true },
430+
},
431+
},
432+
devices: [['d0', require.resolve('./files-devices/device-0'), {}]],
433+
};
434+
const c = await buildVatController(config, ['promise1'], t.context.data);
435+
await c.step();
436+
// if the kernel paniced, that c.step() will reject, and the await will throw
437+
t.deepEqual(c.dump().log, ['sending Promise', 'good: callNow failed']);
438+
});

0 commit comments

Comments
 (0)