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

snphost ok mistakenly report RMP as UNINIT with newest firmware #108

Open
salzg opened this issue Dec 18, 2024 · 7 comments
Open

snphost ok mistakenly report RMP as UNINIT with newest firmware #108

salzg opened this issue Dec 18, 2024 · 7 comments

Comments

@salzg
Copy link

salzg commented Dec 18, 2024

AMD has rolled out new firmware in light of the BadRAM attack going public. (AMD Security Bulletin). The new firmware (Milan 1.55.22, Genoa 1.55.38) changes the STRUCT_PLATFORM_STATUS struct, namely using bit 1 to set the ALIAS_CHECK_COMPLETE flag in the same byte (offset 03h) as the IS_RMP_INIT flag on bit 0.
Currently, snphost ok (/src/ok.rs) checks the entire byte by comparing it to 1 on line 818. With the firmware changes this now needs to either be handled on a bit by bit basis or the comparison value needs to be adjusted to e.g. 3, which would result in folding the alias check into the RMP init check from the perspective of snphost ok.

@tylerfanelli
Copy link
Member

tylerfanelli commented Dec 18, 2024

Thanks, we're discussing this in the sev library now. Will push a fix.

@tylerfanelli
Copy link
Member

@salzg I don't have access to this updated firmware. Can you test with #109?

@salzg salzg changed the title snphosk ok mistakenly report RMP as UNINIT with newest firmware snphost ok mistakenly report RMP as UNINIT with newest firmware Dec 18, 2024
@salzg
Copy link
Author

salzg commented Dec 18, 2024

Thanks, I will test tomorrow!

@salzg
Copy link
Author

salzg commented Dec 19, 2024

snphost ok now successfully reports "RMP INIT: RMP IS INIT". Checked on fw version 24 (Milan).

As a sidenote: The alias check remains unqueried. I do not know whether the AMD FW would report INIT also as failed if the alias check was not successful and unfortunately, I am currently not in a position to force a situation with aliasing. If INIT is not reported as failed, if the aliasing check is not successful, this would require separate handling by snphost ok as well.

@tylerfanelli
Copy link
Member

I've added a query for the alias check. Would you mind rebasing that branch and testing again?

@salzg
Copy link
Author

salzg commented Dec 20, 2024

The alias check exists and it passes. Again, I am not in a position to force an aliasing problem like demonstrated in BadRAM as I am running a BMS on a CSP (and therefore have no hardware access), so I can't test the opposite case.

You have also added the Trusted I/O check as well and it returns a FAIL. This is more or less to be expected as it was also handled by the same byte and snphost ok previously passed (when it was comparing the entire byte). To my understanding, TIO is still in development and should be therefore considered experimental (the latest TIO firmware preview documentation is v0.70 and was published in March '23). Further, TIO must be set to be enabled in the configuration CMDBUF_SNP_INIT_EX for SNP_INIT_EX. As this is handled by the hypervisor, this either needs to be done manually or (I imagine) needs to happen upstream. Both my BMSs (Ubuntu 24.04 with 6.12-sev-snp-amd64 and RHEL 9 with 6.11.0-rc3-snp-host-85ef1ac03941) do not come with TIO enabled out-of-the-box.

@tylerfanelli
Copy link
Member

Ok, since TIO is still in early development, I've removed the check for it (for now). Can you try the series again?

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