|
From: Rhys K. <rhy...@gm...> - 2015-06-28 04:07:57
|
Committed as revision 15361.
On 27 June 2015 at 17:16, Rhys Kidd <rhy...@gm...> wrote:
> Thanks Ian - I'll commit the patch over the weekend.
>
>
> On Saturday, 27 June 2015, Ian Campbell <ij...@he...> wrote:
>
>> On Fri, 2015-06-26 at 18:05 -0700, Bart Van Assche wrote:
>> > On 06/26/15 05:27, Rhys Kidd wrote:
>> > > Proposed patch below for comment, to resolve Coverity report #1233786.
>> > >
>> > > See: coregrind/m_syswrap/syswrap-xen.c
>> > > ==============================
>> > > PRE_MEM_READ("__HYPERVISOR_tmem_op pool_id",
>> > > (Addr)&tmem->pool_id, sizeof(&tmem->pool_id));
>> > >
>> > > Taking the size of &tmem->pool_id, which is the address of an object,
>> is
>> > > suspicious.
>> > >
>> > > It appears that the code should read sizeof(tmem->pool_id). Would a V
>> > > user on xen or at Citrix please be able to test the below proposed
>> patch?
>> > >
>> > > It appears to be a straightforward fix, but would appreciate a more
>> > > knowledgeable user with that platform to confirm.
>> > >
>> > > Regards,
>> > > Rhys
>> > >
>> > >
>> > > Index: coregrind/m_syswrap/syswrap-xen.c
>> > > ===================================================================
>> > > --- coregrind/m_syswrap/syswrap-xen.c (revision 15356)
>> > > +++ coregrind/m_syswrap/syswrap-xen.c (working copy)
>> > > @@ -943,7 +943,7 @@
>> > > * vki_uint32_t subop;
>> > > */
>> > > PRE_MEM_READ("__HYPERVISOR_tmem_op pool_id",
>> > > - (Addr)&tmem->pool_id, sizeof(&tmem->pool_id));
>> > > + (Addr)&tmem->pool_id, sizeof(tmem->pool_id));
>> > > PRE_XEN_TMEMOP_READ(ctrl, subop);
>> > >
>> > > switch (tmem->u.ctrl.subop) {
>> >
>> > Ian or Andrew, can any of you review this patch ?
>>
>> It looks correct to me. The hypervisor side of tmem_op is a bit of a
>> twisty maze but I can't see anything which would explain/justify a need
>> to use sizeof(a pointer) there.
>>
>> Not sure if you need me to say something formal, but:
>> Reviewed-by: Ian Campbell <ian...@ci...>
>>
>> Ian.
>>
>>
|