Re: New Defects reported by Coverity Scan for XenProject

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/04/2025 3:19 pm, Jan Beulich wrote:
> On 02.04.2025 14:56, scan-admin@xxxxxxxxxxxx wrote:
>> ** CID 1645573:  Null pointer dereferences  (FORWARD_NULL)
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 1645573:  Null pointer dereferences  (FORWARD_NULL)
>> /xen/arch/x86/mm/p2m.c: 346 in get_page_from_mfn_and_type()
>> 340     
>> 341         if ( p2m_is_ram(t) )
>> 342         {
>> 343             if ( p2m_is_shared(t) )
>> 344                 d = dom_cow;
>> 345     
>>>>>     CID 1645573:  Null pointer dereferences  (FORWARD_NULL)
>>>>>     Passing null pointer "d" to "get_page", which dereferences it.
>> 346             if ( get_page(page, d) )
>> 347                 return page;
>> 348         }
> I wonder how the tool concludes d is NULL. The only vague guess I could come
> up with is that it checks a MEM_SHARING=n build, where dom_cow is always
> NULL. Yet in such a build p2m_shared() is also always false. Should we perhaps
> help it by forcing P2M_SHARED_TYPES (maybe also P2M_SHARABLE_TYPES) to 0 when
> MEM_SHARING=n? We do such already for MEM_PAGING=n ...

What's in the web UI but missing from the email is:

343        if ( p2m_is_shared(t) )
      4. assign_zero: Assigning: d = NULL.
344            d = dom_cow;
345
     
CID 1645573: (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
5. var_deref_model: Passing null pointer d to get_page, which
dereferences it.[show details]
346        if ( get_page(page, d) )
347            return page;

So it's definitely running a CONFIG_MEM_SHARING=n build, with some logic
that doesn't DCE properly.

I think we'd have a better time if dom_cow remained a dangling extern,
but I expect it's NULL because it doesn't DCE properly.

~Andrew