On Wed, Jun 21, 2017 at 05:13:29PM +0300, Razvan Cojocaru wrote:
> On 06/21/2017 04:58 PM, Wei Liu 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 <ppircal...@bitdefender.com>
> >> Acked-by: Tamas K Lengyel <ta...@tklengyel.com>
> >
> > 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.
> >
> > ** 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;
>
> I vaguely remember that 31 was introduced simply as a "reserved"
> precaution - we can probably safely please Coverity by simply patching
> that code to not go over 3 as an index.
>
> To Petre's credit, he did notice and propose that we change this value
> but I've suggested that we keep the check as-is for the future. My bad. :)
>
OK. Please submit a patch to fix this. Using 3 should be fine. Please
include
Coverity-ID: 1412966
in the commit message.
>
> Thanks,
> Razvan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
