On 23.11.2025 11:01, [email protected] wrote:
> Please find the latest report on new defect(s) introduced to XenProject found
> with Coverity Scan.
>
> 1 new defect(s) introduced to XenProject found with Coverity Scan.
> 3 defect(s), reported by Coverity Scan earlier, were marked fixed in the
> recent build analyzed by Coverity Scan.
>
> New defect(s) Reported-by: Coverity Scan
> Showing 1 of 1 defect(s)
>
>
> ** CID 1675028: Data race undermines locking (LOCK_EVASION)
> /xen/arch/x86/hvm/vmx/vmx.c: 1502 in shadow_handle_cd()
>
>
> _____________________________________________________________________________________________
> *** CID 1675028: Data race undermines locking (LOCK_EVASION)
> /xen/arch/x86/hvm/vmx/vmx.c: 1502 in shadow_handle_cd()
> 1496 }
> 1497 else if ( !(value & X86_CR0_CD) &&
> 1498 (v->arch.hvm.vmx.cache_mode == CACHE_MODE_NO_FILL) )
> 1499 {
> 1500 /* Exit from no fill cache mode. */
> 1501 spin_lock(&d->arch.hvm.vmx.uc_lock);
>>>> CID 1675028: Data race undermines locking (LOCK_EVASION)
>>>> The thread 1 sets "cache_mode" to a new value. Now the two threads
>>>> have an inconsistent view of "cache_mode" and updates to fields correlated
>>>> with "cache_mode" may be lost.
> 1502 v->arch.hvm.vmx.cache_mode = CACHE_MODE_NORMAL;
> 1503
> 1504 if ( domain_exit_uc_mode(v) )
> 1505 set_uc_mode(d, false);
> 1506
> 1507 spin_unlock(&d->arch.hvm.vmx.uc_lock);
This clearly isn't a new one, but merely flagged as such because the code moved.
There look to be at least two aspects. For one, the tool may be irritated by
shadow and HAP modes being handled differently: In the former updates (and
corresponding checks) happen under lock, while in the latter case they don't.
This looks to leverage that no domain-wide checking is necessary, and hence
updates only ever occurring on the running vCPU means updating is race-free
despite being lock-less. (Sadly parameter / variable naming doesn't properly
reflect this "on current" aspect, despite it being pretty important here.)
Further there are vmx_{get,set}_guest_pat(), which inspect the field without
holding the lock. These functions are intended to act on either current or on
paused vCPU-s / domains only, though. Hence no race either due to all updating
being done on current.
Am I overlooking anything, or does this simply need silencing (again) in the
tool?
Jan
