Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

slots in vatAdmin terminate/createVat are not translated correctly #4588

Closed
warner opened this issue Feb 17, 2022 · 1 comment · Fixed by #4872
Closed

slots in vatAdmin terminate/createVat are not translated correctly #4588

warner opened this issue Feb 17, 2022 · 1 comment · Fixed by #4872
Assignees
Labels
bug Something isn't working SwingSet package: SwingSet
Milestone

Comments

@warner
Copy link
Member

warner commented Feb 17, 2022

While working on #4566 I realized that we're probably mishandling object references (slots) in the "terminate vat from the outside" case:

// this is vat 1
  const { admin } = await E(vatAdminService).createVat(bundlecap);
  const obj = Far('iface', {});
  const reason = [ 'data', obj ]; // usually an Error, but anything is legal
  E(admin).terminateWithFailure(reason);

The object obj is assigned a vat-1 vref as terminateWithFailure gets serialized for syscall.send, then assigned a kref as the syscall is translated. When the message is delivered to vat-admin, the c-list allocates a vat-admin vref, which is what appears at the vatAdminWrapper.js terminateWithFailure() call:

    const adminNode = Far('adminNode', {
      terminateWithFailure(reason) {
        D(vatAdminNode).terminateWithFailure(vatID, reason);
      },

The D(vatAdminNode) does a syscall.callNow, which translates the vref through the kref into a devices.vatAdmin dref, which is what arrives at the device code.

Previously (on current trunk), the device code was written with deviceSlots, which provided a copy of its m.serialize to the device code, so vatAdmin-src.js could re-serialize the reason object into capdata. vatAdmin-src.js then passed the reason capdata to its kernel endowment (terminate(vatID, serialize(reason))).

The problem is that serialize(reason) is device-side capdata, with "drefs" (the device equivalent of "vrefs" in a vat). But the kernel terminate endowment is going to use that reason capdata to reject the admin.done() promise: the kernel enqueues a vatTerminated message to vat-admin, and queueToKref assumes that the arguments you give it use krefs.

The consequence is that when we attempt to deliver the vatTerminated to vat-admin, the translator will encounter invalid slot types. This will either cause a kernel panic, or cause the failure of that message (and admin.done() will never fire), probably the former.

In my #4566 rewrite, I encountered the same problem in a raw-device form of vatAdmin-src.js. I get dref capdata from the dispatch.invoke, and I want to provide kref capdata to the kernel endowment.

The same problem exists in createVat with the options argument. This one is more pressing because #4381 will want to include bundlecaps in options.vatParameters.

For now, I'm adding an assertion that the .slots are empty, so we catch this in the calling vat. For most of these methods that's going to be vatAdminWrapper.js, which will probably cause the terminateWithFailure() or createVat() to fail, which is a perfectly reasonable way to handle it. But to implement #4381 we'll need something better.

It probably wants to be a syscall, because that provides an easy place to perform dref-to-kref translation. But after translation, we want to hit some kernel-provided API. The terminate case has no return value, but the createVat cases want to return a vatID from that API.

This might overlap with #720 (a kernel input queue). One thought is that the built-in device might be given an endowment which is a marker/handle object (one for terminate, another one or two for createVat/createVatByBundle). We add a syscall.triggerKernelSomething(handle, deviceCapdata), the syscall translates deviceCapdata into kernelCapdata, then invokes whatever function the kernel had previously registered with handle (possibly through a queue that protects the kernel from reentrancy).

The way this interacts with #720 would be that external callers could invoke the device's exterior APIs any time they want, and syscall.triggerKernelSomething would know to queue the actual translation and invocation until the kernel was safely idle. I'm not sure that's sound.

@warner warner added bug Something isn't working SwingSet package: SwingSet labels Feb 17, 2022
@warner warner self-assigned this Feb 17, 2022
@Tartuffo
Copy link
Contributor

Chip's latest may make only one vat crash.

warner added a commit that referenced this issue Mar 19, 2022
This allows `E(vatAdminService).createVat(bundleID, { vatParameters })` to
include object references in `vatParameters`, which are then delivered
through the `dispatch.startVat()` delivery and made available to
`buildRootObject(vatPowers, vatParameters)`.

From within a vat, `vatPowers.exitVat(completion)` and
`.exitVatWithFailure(reason)` can include object references, and they will be
delivered to the parent caller's `done` promise.

From outside the vat, `E(adminNode).terminateWithFailure(reason)` can take
object refs in `reason` and they will be delivered to the parent caller's
`done` promise. `E(adminNode).upgrade(bundleID, vatParameters)` can take
object refs in `vatParameters` just like `createVat` (although `upgrade`
itself is still non-functional).

The kernel will maintain a refcount on each object passed through this
mechanism, to keep it from being collected while in transit.

This is implemented with the new "kernel device hooks" feature, which allows
a device to call into the kernel and have its drefs translated into krefs.

This will help with ZCF/contract vat upgrade, to pass the new contract
bundlecap into the new ZCF vat via vatParameters.

closes #4588
closes #4381
refs #1848
warner added a commit that referenced this issue Mar 19, 2022
This allows `E(vatAdminService).createVat(bundleID, { vatParameters })` to
include object references in `vatParameters`, which are then delivered
through the `dispatch.startVat()` delivery and made available to
`buildRootObject(vatPowers, vatParameters)`.

In vat-vat-admin, the CreateVatOptions validation was rewritten to
ensure that any existing slots are only in vatParameters.

From within a vat, `vatPowers.exitVat(completion)` and
`.exitVatWithFailure(reason)` can include object references, and they will be
delivered to the parent caller's `done` promise.

From outside the vat, `E(adminNode).terminateWithFailure(reason)` can take
object refs in `reason` and they will be delivered to the parent caller's
`done` promise. `E(adminNode).upgrade(bundleID, vatParameters)` can take
object refs in `vatParameters` just like `createVat` (although `upgrade`
itself is still non-functional).

The kernel will maintain a refcount on each object passed through this
mechanism, to keep it from being collected while in transit.

This is implemented with the new "kernel device hooks" feature, which allows
a device to call into the kernel and have its drefs translated into
krefs.

This will help with ZCF/contract vat upgrade, to pass the new contract
bundlecap into the new ZCF vat via vatParameters.

closes #4588
closes #4381
refs #1848
warner added a commit that referenced this issue Mar 22, 2022
This allows `E(vatAdminService).createVat(bundleID, { vatParameters })` to
include object references in `vatParameters`, which are then delivered
through the `dispatch.startVat()` delivery and made available to
`buildRootObject(vatPowers, vatParameters)`.

In vat-vat-admin, the CreateVatOptions validation was rewritten to
ensure that any existing slots are only in vatParameters.

From within a vat, `vatPowers.exitVat(completion)` and
`.exitVatWithFailure(reason)` can include object references, and they will be
delivered to the parent caller's `done` promise.

From outside the vat, `E(adminNode).terminateWithFailure(reason)` can take
object refs in `reason` and they will be delivered to the parent caller's
`done` promise. `E(adminNode).upgrade(bundleID, vatParameters)` can take
object refs in `vatParameters` just like `createVat` (although `upgrade`
itself is still non-functional).

The kernel will maintain a refcount on each object passed through this
mechanism, to keep it from being collected while in transit.

This is implemented with the new "kernel device hooks" feature, which allows
a device to call into the kernel and have its drefs translated into
krefs.

This will help with ZCF/contract vat upgrade, to pass the new contract
bundlecap into the new ZCF vat via vatParameters.

closes #4588
closes #4381
refs #1848
@mergify mergify bot closed this as completed in #4872 Mar 22, 2022
@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants