Re: New Defects reported by Coverity Scan for XenProject

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.07.2026 15:21, Andrew Cooper wrote:
> On 30/07/2026 2:14 pm, Jan Beulich wrote:
>> On 30.07.2026 12:24, scan-admin@xxxxxxxxxxxx wrote:
>>> ** CID 1697485:       Control flow issues  (DEADCODE)
>>> /xen/arch/x86/hvm/hvm.c: 2436           in hvm_set_cr3()
>>>
>>>
>>> _____________________________________________________________________________________________
>>> *** CID 1697485:         Control flow issues  (DEADCODE)
>>> /xen/arch/x86/hvm/hvm.c: 2436             in hvm_set_cr3()
>>> 2430                 curr->arch.vm_event->write_data.cr3_noflush = noflush;
>>> 2431     
>>> 2432                 return X86EMUL_OKAY;
>>> 2433             }
>>> 2434         }
>>> 2435     
>>>>>>     CID 1697485:         Control flow issues  (DEADCODE)
>>>>>>     Execution cannot reach the expression "(value ^ 
>>>>>> curr->arch.hvm.guest_cr[3]) >> 12" inside this statement: "if 
>>>>>> (!!(curr->arch.hvm.guest...".
>>> 2436         if ( hvm_paging_enabled(curr) && paging_mode_shadow(currd) &&
>>> 2437              ((value ^ curr->arch.hvm.guest_cr[3]) >> PAGE_SHIFT) )
>>> 2438         {
>>> 2439             /* Shadow-mode CR3 change. Check PDBR and update 
>>> refcounts. */
>>> 2440             struct page_info *page;
>>> 2441     
>>>
>>> ** CID 1697484:       Control flow issues  (DEADCODE)
>>> /xen/arch/x86/hvm/svm/svm.c: 128           in svm_update_guest_cr()
>>>
>>>
>>> _____________________________________________________________________________________________
>>> *** CID 1697484:         Control flow issues  (DEADCODE)
>>> /xen/arch/x86/hvm/svm/svm.c: 128             in svm_update_guest_cr()
>>> 122                      monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
>>> 123                    vmcb_set_cr_intercepts(vmcb, intercepts | 
>>> CR_INTERCEPT_CR3_WRITE);
>>> 124             }
>>> 125     
>>> 126             value = v->arch.hvm.guest_cr[0];
>>> 127             if ( paging_mode_shadow(v->domain) )
>>>>>>     CID 1697484:         Control flow issues  (DEADCODE)
>>>>>>     Execution cannot reach this statement: "value |= 0x80010000UL;".
>>> 128                 value |= X86_CR0_PG | X86_CR0_WP;
>>> 129             vmcb_set_cr0(vmcb, value);
>>> 130             break;
>>> 131     
>>> 132         case 2:
>>> 133             vmcb_set_cr2(vmcb, v->arch.hvm.guest_cr[2]);
>>>
>>> ** CID 1697483:       Control flow issues  (DEADCODE)
>>> /xen/arch/x86/hvm/svm/svm.c: 396           in svm_vmcb_restore()
>>>
>>>
>>> _____________________________________________________________________________________________
>>> *** CID 1697483:         Control flow issues  (DEADCODE)
>>> /xen/arch/x86/hvm/svm/svm.c: 396             in svm_vmcb_restore()
>>> 390                 return -EINVAL;
>>> 391             }
>>> 392         }
>>> 393     
>>> 394         if ( paging_mode_shadow(v->domain) )
>>> 395         {
>>>>>>     CID 1697483:         Control flow issues  (DEADCODE)
>>>>>>     Execution cannot reach this statement: "if (c->cr0 & 0x80000000UL) 
>>>>>> ...".
>>> 396             if ( c->cr0 & X86_CR0_PG )
>>> 397             {
>>> 398                 page = get_page_from_gfn(v->domain, c->cr3 >> 
>>> PAGE_SHIFT,
>>> 399                                          NULL, P2M_ALLOC);
>>> 400                 if ( !page )
>>> 401                 {
>>>
>>> ** CID 1697482:         (DEADCODE)
>>> /xen/arch/x86/hvm/hvm.c: 2332           in hvm_set_cr0()
>>> /xen/arch/x86/hvm/hvm.c: 2382           in hvm_set_cr0()
>>>
>>>
>>> _____________________________________________________________________________________________
>>> *** CID 1697482:           (DEADCODE)
>>> /xen/arch/x86/hvm/hvm.c: 2332             in hvm_set_cr0()
>>> 2326                 hvm_update_guest_efer(v);
>>> 2327             }
>>> 2328     
>>> 2329             if ( paging_mode_shadow(d) )
>>> 2330             {
>>> 2331                 /* The guest CR3 must be pointing to the guest 
>>> physical. */
>>>>>>     CID 1697482:           (DEADCODE)
>>>>>>     Execution cannot reach this statement: "gfn = 
>>>>>> v->arch.hvm.guest_cr[...".
>>> 2332                 gfn = v->arch.hvm.guest_cr[3] >> PAGE_SHIFT;
>>> 2333                 page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
>>> 2334                 if ( !page )
>>> 2335                 {
>>> 2336                     gdprintk(XENLOG_ERR, "Invalid CR3 value = %lx\n",
>>> 2337                              v->arch.hvm.guest_cr[3]);
>>> /xen/arch/x86/hvm/hvm.c: 2382             in hvm_set_cr0()
>>> 2376                 v->arch.hvm.guest_efer &= ~EFER_LMA;
>>> 2377                 hvm_update_guest_efer(v);
>>> 2378             }
>>> 2379     
>>> 2380             if ( paging_mode_shadow(d) )
>>> 2381             {
>>>>>>     CID 1697482:           (DEADCODE)
>>>>>>     Execution cannot reach this statement: "put_page((struct page_info 
>>>>>> ...".
>>> 2382                 put_page(pagetable_get_page(v->arch.guest_table));
>>> 2383                 v->arch.guest_table = pagetable_null();
>>> 2384             }
>>> 2385         }
>>> 2386     
>>> 2387         if ( ((value ^ old_value) & X86_CR0_CD) &&
>> For all of the above (and maybe more further down the original report): Do 
>> we have
>> control over the .config Coverity uses? These (not really) issues all result 
>> from
>> SHADOW_PAGING=n by default now, i.e. the unreachable statements are expected 
>> in
>> that configuration (and they were there before, if the build had been done 
>> with
>> SHADOW_PAGING=n).
> 
> Yes.  It is .github/workflows/coverity.yml which does the build and
> submission to Coverity.
> 
> Right now it's a plain `make build-xen`, so will go from absolute
> defaults, but we can change it if needs be.

So first question: Which exact way do we want the Coverity scan to be done? Is 
there
a reason we have it use a plain default config (which can change at any time, 
as has
happened now), rather than e.g. an allyesconfig one? Or something yet further
customized (e.g. in xen/arch/*/configs/coverity_defconfig)?

It's only going to be a follow-up question then how exactly to make changes to 
that
file you point at. I, for one, have no insight into how to try out changes being
made, before actually sending out a patch.

Jan