Re: [Xen-devel] [PATCH v4 1/2] x86/monitor: add masking support for write_ctrlreg events

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Wed, Jun 21, 2017 at 7:58 AM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> On Mon, Jun 19, 2017 at 03:24:38PM +0300, Petre Pircalabu wrote:
>> Add support for filtering out the write_ctrlreg monitor events if they
>> are generated only by changing certains bits.
>> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
>> function in order to mask the event generation if the changed bits are
>> set.
>>
>> Signed-off-by: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
>> Acked-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
>
> Coverity isn't happy with this patch.
>
> It seems to me there is indeed a risk to overrun the buffer (4 in size) 
> because
> the caller can specify index up to 31.

Indeed. We have a sanity check earlier in here that checks whether
index > 31 but it would make more sense to check it against the max
valid value of index to begin with (which at the moment is
VM_EVENT_X86_XCR0 = 3).

>
> ** CID 1412966:  Memory - corruptions  (OVERRUN)
> /xen/arch/x86/monitor.c: 162 in arch_monitor_domctl_event()
>
>
> ________________________________________________________________________________________________________
> *** CID 1412966:  Memory - corruptions  (OVERRUN)
> /xen/arch/x86/monitor.c: 162 in arch_monitor_domctl_event()
> 156                 ad->monitor.write_ctrlreg_onchangeonly |= ctrlreg_bitmask;
> 157             else
> 158                 ad->monitor.write_ctrlreg_onchangeonly &= 
> ~ctrlreg_bitmask;
> 159
> 160             if ( requested_status )
> 161             {
>>>>     CID 1412966:  Memory - corruptions  (OVERRUN)
>>>>     Overrunning array "ad->monitor.write_ctrlreg_mask" of 4 8-byte 
>>>> elements at element index 31 (byte offset 248) using index 
>>>> "mop->u.mov_to_cr.index"
> (which evaluates to 31).
> 162                 ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 
> mop->u.mov_to_cr.bitmask;
> 163                 ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
> 164             }
> 165             else
> 166             {
> 167                 ad->monitor.write_ctrlreg_mask[mop->u.mov_to_cr.index] = 
> 0;

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel