Skip to content

Commit

Permalink
fix: reject promises in the arguments to syscall.callNow()
Browse files Browse the repository at this point in the history
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
  • Loading branch information
warner committed Sep 15, 2020
1 parent 0905007 commit 7472661
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 0 deletions.
6 changes: 6 additions & 0 deletions packages/SwingSet/src/kernel/vatTranslator.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ function makeTranslateVatSyscallToKernelSyscall(vatID, kernelKeeper) {
if (type !== 'device') {
throw new Error(`doCallNow must target a device, not ${dev}`);
}
for (const slot of args.slots) {
assert(
parseVatSlot(slot).type !== 'promise',
`syscall.callNow() args cannot include promises like ${slot}`,
);
}
const kernelSlots = args.slots.map(slot => mapVatSlotToKernelSlot(slot));
const kernelData = harden({ ...args, slots: kernelSlots });
// prettier-ignore
Expand Down
12 changes: 12 additions & 0 deletions packages/SwingSet/test/files-devices/bootstrap-2.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,18 @@ export function buildRootObject(vatPowers, vatParameters) {
},
});
D(devices.command).registerInboundHandler(handler);
} else if (argv[0] === 'promise1') {
const p = Promise.resolve();
log('sending Promise');
try {
// this will be rejected immediately, killing the vat shortly
D(devices.d0).send({ p });
// shouldn't get here
log('oops: survived sending Promise');
} catch (e) {
// we aren't currently killed until the end of the crank
log('good: callNow failed');
}
} else {
throw new Error(`unknown argv mode '${argv[0]}'`);
}
Expand Down
17 changes: 17 additions & 0 deletions packages/SwingSet/test/test-devices.js
Original file line number Diff line number Diff line change
Expand Up @@ -419,3 +419,20 @@ test.serial('command deliver', async t => {
t.deepEqual(c.dump().log, ['handle-0-missing', 'handle-1-errory']);
t.deepEqual(rejection, { response: 'body' });
});

test('callNow refuses promises', async t => {
const config = {
bootstrap: 'bootstrap',
vats: {
bootstrap: {
bundle: t.context.data.bootstrap2,
creationOptions: { enableSetup: true },
},
},
devices: [['d0', require.resolve('./files-devices/device-0'), {}]],
};
const c = await buildVatController(config, ['promise1'], t.context.data);
await c.step();
// if the kernel paniced, that c.step() will reject, and the await will throw
t.deepEqual(c.dump().log, ['sending Promise', 'good: callNow failed']);
});

0 comments on commit 7472661

Please sign in to comment.