|
From: Rhys K. <rhy...@gm...> - 2015-06-26 12:27:18
|
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) {
|
|
From: Rhys K. <rhy...@gm...> - 2015-06-27 07:16:42
|
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... <javascript:;>>
>
> Ian.
>
>
|
|
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.
>>
>>
|