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

Malfunctioning SEV-SNP device results in nil-pointer exception #89

Closed
msanft opened this issue Oct 4, 2023 · 6 comments
Closed

Malfunctioning SEV-SNP device results in nil-pointer exception #89

msanft opened this issue Oct 4, 2023 · 6 comments

Comments

@msanft
Copy link
Contributor

msanft commented Oct 4, 2023

Hey there!

If abi.SevProduct() is called in a guest with a malfunctioning SEV-SNP device, this results in the cpuid() call to error with a nil-pointer exception as follows (line numbers based on v0.8.0 this library):

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x411ded9]
goroutine 170 [running]:
github.com/google/go-sev-guest/abi.SevProduct()
external/com_github_google_go_sev_guest/abi/abi.go:810 +0x19
github.com/google/go-sev-guest/client.(*LinuxDevice).Product(0xc00068c580?)
external/com_github_google_go_sev_guest/client/client_linux.go:124 +0xf
github.com/google/go-sev-guest/client.GetExtendedReportAtVmpl({0x15728d0, 0xc000c59a10}, {0xca, 0xcb, 0x75, 0x2, 0xfd, 0x96, 0x8, 0x32, ...}, ...)
external/com_github_google_go_sev_guest/client/client.go:180 +0xf9
github.com/google/go-sev-guest/client.GetExtendedReport(...)
external/com_github_google_go_sev_guest/client/client.go:186

While I still need to figure out what in cpuid exactly causes this failure (haven't got to read into the spec of CPUID yet), I think an easy approach would be to recover from the panic and return an error in that case that could be passed on to the caller, which would lead to an early failure and thus be my personal favorite. Otherwise, falling back to the DefaultSevProduct might be an option too if the function's signature should be kept as is.

Feel free to let me know your thoughts on this and I will be happy to implement it!

@deeglaze
Copy link
Collaborator

deeglaze commented Oct 4, 2023

Release v0.9.1 includes a fix for this. Apologies for the breakage.

@deeglaze deeglaze closed this as completed Oct 4, 2023
@msanft
Copy link
Contributor Author

msanft commented Oct 5, 2023

Hey @deeglaze,
Super sorry to ping you here again. Judging from #83, you've ran into the same problem as we did? I think that with the fix, you would get a product with Name: Unknown, Stepping: 0. Is this a behaviour that you want? Some other (Google) libraries such as go-tpm-tools rely on an error being returned in case of a failure.

Essentially, the current way of handling this in v0.9.1 wouldn't return an error, but might return an incomplete / invalid report struct, which leads to other errors in the library. While this could certainly be handled in go-tpm-tools as well, I would like to hear your opinion on handling this in go-sev-guest directly.

Thank you and sorry for interrupting again with this!

@deeglaze
Copy link
Collaborator

deeglaze commented Oct 5, 2023

Would you mind sharing your cpuid_0000_0001_EAX value you get in the guest? I would expect for a Milan-B0 it should be 0x00a00f10.

Edit: Milan-B1 would be 0x00a00f11.

@msanft
Copy link
Contributor Author

msanft commented Oct 5, 2023

Thanks for the fast reply! I will conduct a test tomorrow an let you know. We currently suspect a problem with the CSP's hypervisor that causes corrupted {Reports, CPUID responses}, but didn't yet check the exact value. I will let you know here as soon as i figure something out.

@deeglaze
Copy link
Collaborator

deeglaze commented Oct 5, 2023

That wouldn't surprise me. We have a big file of hardcoded CPUID values to use for VMs depending on CPU make/model.

@msanft
Copy link
Contributor Author

msanft commented Oct 9, 2023

Hey @deeglaze ,

Sorry for the confusion. I've managed to clear this out now. The CPUID EAX was actually 0xa00f10, which should be correct. It turned out to be an issue with go-tpm-tools rather than go-sev-guest. Thanks a lot for your support!

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

No branches or pull requests

2 participants