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

Check CPUID instruction in Stage0 #VC handler #4921

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

conradgrobler
Copy link
Collaborator

We want to make sure that the instruction pointer in a #VC exception really pointed to a CPUID instruction since it is the only #VC exception type we support.

Ref b/330197837

@conradgrobler conradgrobler requested a review from andrisaar March 20, 2024 17:01
@@ -43,7 +43,10 @@ gp_handler:
vc_handler:
pop %ebx # get the error code
cmp $0x72, %ebx # is this about CPUID?
jne 2f # if not, skip ahead
jne 2f # if not, skip ahead and crash
pop %ebx # get the instruction pointer
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pops the EIP value off the stack, I think that'll break the IRET later (where should it return to?).

What you'd really want to do is effectively ((%esp)) (first dereference ESP to get the value of EIP, then dereference EIP) but I'm pretty sure you can't do it like that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, good point.

@conradgrobler conradgrobler merged commit ef2c4ec into project-oak:main Mar 21, 2024
24 checks passed
@conradgrobler conradgrobler deleted the stage0-vc branch March 21, 2024 11:02
waywardgeek pushed a commit to waywardgeek/oak that referenced this pull request Mar 21, 2024
We want to make sure that the instruction pointer in a #VC exception really pointed to a CPUID instruction since it is the only #VC exception type we support.

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

Successfully merging this pull request may close these issues.

2 participants