Re: New Defects reported by Coverity Scan for XenProject

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.06.2026 23:53, Jason Andryuk wrote:
> On 2026-06-24 09:04, Jan Beulich wrote:
>> On 24.06.2026 14:33, scan-admin@xxxxxxxxxxxx wrote:
>>> ** CID 1695359:       Insecure data handling  (INTEGER_OVERFLOW)
>>> /tools/xenstored/domain.c: 601           in domain_tree_remove_sub()
>>>
>>>
>>> _____________________________________________________________________________________________
>>> *** CID 1695359:         Insecure data handling  (INTEGER_OVERFLOW)
>>> /tools/xenstored/domain.c: 601             in domain_tree_remove_sub()
>>> 595                 node_changed = true;
>>> 596         }
>>> 597
>>> 598         for (i = 1; i < node->hdr.num_perms; i++) {
>>> 599                 if (node->perms[i].id != domain->domid)
>>> 600                         continue;
>>>>>>      CID 1695359:         Insecure data handling  (INTEGER_OVERFLOW)
>>>>>>      "8UL * (node->hdr.num_perms - i - 1U)", which might have 
>>>>>> underflowed, is passed to "memmove(node->perms + i, node->perms + i + 1, 
>>>>>> 8UL * (node->hdr.num_perms - i - 1U))". [Note: The source code 
>>>>>> implementation of the function has been overridden by a builtin model.]
>>> 601                 memmove(node->perms + i, node->perms + i + 1,
>>> 602                         sizeof(*node->perms) * (node->hdr.num_perms - i 
>>> - 1));
>>
>> I'm struggling with this one: As i < node->hdr.num_perms, the last argument
>> passed to memmove() can be 0, but I can't see potential for underflow.
> 
> This gave me pause on my initial review.  On the final iteration,
> node->perms + i + 1 will point past the end of the allocation,

To be precise, it may point _at_ the end of the allocation (i.e. immediately
past the last element), and hence still not UB.

> but as 
> you say the size would be 0.  I originally considered suggesting a check 
> and then decided it was unnecessary because of the 0.

If Coverity's wording is to be trusted, such a check also wouldn't have
helped here because 0 still is "no underflow".

Jan