Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] This problem wasn't introduced by this patch, but a malicious hypervisor can bypass Secure TSC calibration by injecting a fake UV system table, forcing the guest to fall back to untrusted legacy timers. - [High] This problem wasn't introduced by this patch, but if the calculated Secure TSC frequency is 0, the kernel silently falls back to untrusted timers instead of terminating the VM. -- Note: Due to system recitation restrictions, this report is provided as a free-form summary without direct diff quoting. commit cdbd80de837f6d512a759452a18164ddd83b9b7b Author: Sean Christopherson <[email protected]> x86/sev: Mark TSC as reliable when configuring Secure TSC This commit moves the setting of the X86_FEATURE_TSC_RELIABLE flag from sme_early_init() to snp_secure_tsc_init(). It consolidates the handling of known-frequency and reliable TSC flags when overriding the TSC calibration routine. [Severity: High] This isn't a bug introduced by this patch, but does the early return in arch/x86/kernel/tsc.c:tsc_early_init() allow an untrusted hypervisor to bypass Secure TSC initialization? The tsc_early_init() function checks is_early_uv_system() before initializing Secure TSC. The is_early_uv_system() check relies on the presence of UV_SYSTEM_TABLE_GUID in the EFI configuration tables. Because the hypervisor can provide tables to the guest firmware which may expose them as EFI configuration tables, an untrusted hypervisor could inject this specific GUID. Would this trick the kernel into returning early, bypassing both Secure TSC and TDX TSC initialization, and causing the guest to fall back to untrusted legacy timers? [Severity: High] This is also a pre-existing issue, but should the newly updated arch/x86/coco/sev/core.c:snp_secure_tsc_init() validate that the calculated frequency is non-zero before returning? If the calculated frequency evaluates to 0, which could happen if the hypervisor causes MSR_AMD64_GUEST_TSC_FREQ to read as 0 or if the secrets factor is invalid, the function simply returns 0 instead of terminating the VM. The caller tsc_early_init() treats a return value of 0 as a failure to find a trusted frequency and gracefully falls back to x86_init.hyper.get_tsc_khz() or legacy timers. In a confidential computing threat model, should this security-critical initialization fail closed and terminate the VM, rather than falling back to the hypervisor-controlled time sources that Secure TSC is designed to protect against? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
