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

libsel4vm: set VM_GUEST_ERROR_EXIT on vCPU index error? #47

Open
axel-h opened this issue May 3, 2022 · 6 comments
Open

libsel4vm: set VM_GUEST_ERROR_EXIT on vCPU index error? #47

axel-h opened this issue May 3, 2022 · 6 comments
Labels

Comments

@axel-h
Copy link
Member

axel-h commented May 3, 2022

I wonder, in

tag = seL4_Recv(vm->host_endpoint, &sender_badge);
label = seL4_MessageInfo_get_label(tag);
if (sender_badge >= MIN_VCPU_BADGE && sender_badge <= MAX_VCPU_BADGE) {
seL4_Word vcpu_idx = VCPU_BADGE_IDX(sender_badge);
if (vcpu_idx >= vm->num_vcpus) {
ZF_LOGE("Invalid VCPU index. Exiting");
ret = -1;
} else {
shouldn't this also set

vm->run.exit_reason = VM_GUEST_ERROR_EXIT;

if vcpu_idx is invalid?

@axel-h axel-h added the question label May 3, 2022
@Ivan-Velickovic
Copy link
Contributor

Is there an instance where exit_reason is actually used? I can only see it being set.

@axel-h
Copy link
Member Author

axel-h commented May 9, 2022

I though it's part of the API. So the examples should be improved to make use of this - or have a comment that they don't use this because it does not make an difference for the example.

@alisonfel
Copy link

I wonder, in

tag = seL4_Recv(vm->host_endpoint, &sender_badge);
label = seL4_MessageInfo_get_label(tag);
if (sender_badge >= MIN_VCPU_BADGE && sender_badge <= MAX_VCPU_BADGE) {
seL4_Word vcpu_idx = VCPU_BADGE_IDX(sender_badge);
if (vcpu_idx >= vm->num_vcpus) {
ZF_LOGE("Invalid VCPU index. Exiting");
ret = -1;
} else {

shouldn't this also set

vm->run.exit_reason = VM_GUEST_ERROR_EXIT;

if vcpu_idx is invalid?

Yep, vm->run.exit_reason = VM_GUEST_ERROR_EXIT; should be set when the vcpu_idx is invalid. This is likely a small bug, I might've just missed this when refactoring the libraries a few years ago.

@axel-h
Copy link
Member Author

axel-h commented May 9, 2022

I assume it should also be set here then:

if (vm->run.notification_callback) {
err = vm->run.notification_callback(vm, sender_badge, tag,
vm->run.notification_callback_cookie);
} else {
ZF_LOGE("Unable to handle VM notification. Exiting");
err = -1;
}

@alisonfel
Copy link

Hmm, I don't follow? That case should be already handled just below:

if (err) {
ret = -1;
vm->run.exit_reason = VM_GUEST_ERROR_EXIT;
}

@axel-h
Copy link
Member Author

axel-h commented May 9, 2022

Ah yes, it's set there eventually for all errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants