On Wed, Jul 01, 2026, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) 
> to consider:
> 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?

I suppose so?  But that's a problem best handled in a dedicated, separate patch.

> 
> [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 hypervisor can't do that?  If it can, that too should be addressed 
separately.

> 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

Reply via email to