Uninitialized variables in hvm_event_breakpoint (Re: New Defects reported by Coverity Scan for XenProject)

Uninitialized variables in hvm_event_breakpoint (Re: New Defects reported by Coverity Scan for XenProject)

[Ian Campbell]

On Wed, 2016-02-17 at 16:02 -0800, scan-admin@coverity.com wrote:
> Hi,
> 
> Please find the latest report on new defect(s) introduced to XenProject
> found with Coverity Scan.
> 
> 1 new defect(s) introduced to XenProject found with Coverity Scan.
> 4 defect(s), reported by Coverity Scan earlier, were marked fixed in the
> recent build analyzed by Coverity Scan.
> 
> New defect(s) Reported-by: Coverity Scan
> Showing 1 of 1 defect(s)
> 
> 
> ** CID 1353192:  Uninitialized variables  (UNINIT)
> /xen/arch/x86/hvm/event.c: 176 in hvm_event_breakpoint()

This appears to have been introduced by: 
    commit
    557c7873f35aa39bd84977b28948457b1b342f92
    Author: Corneliu ZUZU <czuzu@bitdef
    ender.com>
    Date:   Mon Feb 15 14:14:16 2016 +0100

        x86: merge 2 hvm_event_... functions into 1
        
        This patch merges almost identical functions hvm_event_int3 and
        hvm_event_single_step into a single function called hvm_event_breakpoint.
        Also fixes event.c file header comment in the process.
        
        Signed-off-by: Corneliu ZUZU <    czuzu@bitdefender.com    >
        Acked-by: Razvan Cojocaru <    rcojocaru@bitdefender.com    >
        Acked-by: Jan Beulich <    jbeulich@suse.com    >


hvm_event_breakpoint calls hvm_event_traps(&req) and if sync is true that
ors some bits into req->flags which was never initialised.

> 
> 
> _________________________________________________________________________
> _______________________________
> *** CID 1353192:  Uninitialized variables  (UNINIT)
> /xen/arch/x86/hvm/event.c: 176 in hvm_event_breakpoint()
> 170     
> 171     int hvm_event_breakpoint(unsigned long rip,
> 172                              enum hvm_event_breakpoint_type type)
> 173     {
> 174         struct vcpu *curr = current;
> 175         struct arch_domain *ad = &curr->domain->arch;
> > > >     CID 1353192:  Uninitialized variables  (UNINIT)
> > > >     Declaring variable "req" without initializer.
> 176         vm_event_request_t req;
> 177     
> 178         switch ( type )
> 179         {
> 180         case HVM_EVENT_SOFTWARE_BREAKPOINT:
> 181             if ( !ad->monitor.software_breakpoint_enabled )
> 
> 
> _________________________________________________________________________
> _______________________________
> To view the defects in Coverity Scan visit, https://scan.coverity.com/pro
> jects/xenproject?tab=overview
> 
> To manage Coverity Scan email notifications for
> "ian.campbell@citrix.com", click https://scan.coverity.com/subscriptions/
> edit?email=ian.campbell%40citrix.com&token=1ce0fc428b9f94f66fd8d1ecf6cbb7
> 6a
> 

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

Re: Uninitialized variables in hvm_event_breakpoint (Re: New Defects reported by Coverity Scan for XenProject)

[Corneliu ZUZU]

On 2/18/2016 12:01 PM, Ian Campbell wrote:
> On Wed, 2016-02-17 at 16:02 -0800, scan-admin@coverity.com wrote:
>> Hi,
>>
>> Please find the latest report on new defect(s) introduced to XenProject
>> found with Coverity Scan.
>>
>> 1 new defect(s) introduced to XenProject found with Coverity Scan.
>> 4 defect(s), reported by Coverity Scan earlier, were marked fixed in the
>> recent build analyzed by Coverity Scan.
>>
>> New defect(s) Reported-by: Coverity Scan
>> Showing 1 of 1 defect(s)
>>
>>
>> ** CID 1353192:  Uninitialized variables  (UNINIT)
>> /xen/arch/x86/hvm/event.c: 176 in hvm_event_breakpoint()
> This appears to have been introduced by:
>      commit
>      557c7873f35aa39bd84977b28948457b1b342f92
>      Author: Corneliu ZUZU <czuzu@bitdef
>      ender.com>
>      Date:   Mon Feb 15 14:14:16 2016 +0100
>
>          x86: merge 2 hvm_event_... functions into 1
>          
>          This patch merges almost identical functions hvm_event_int3 and
>          hvm_event_single_step into a single function called hvm_event_breakpoint.
>          Also fixes event.c file header comment in the process.
>          
>          Signed-off-by: Corneliu ZUZU <    czuzu@bitdefender.com    >
>          Acked-by: Razvan Cojocaru <    rcojocaru@bitdefender.com    >
>          Acked-by: Jan Beulich <    jbeulich@suse.com    >
>
>
> hvm_event_breakpoint calls hvm_event_traps(&req) and if sync is true that
> ors some bits into req->flags which was never initialised.
>
>>
>> _________________________________________________________________________
>> _______________________________
>> *** CID 1353192:  Uninitialized variables  (UNINIT)
>> /xen/arch/x86/hvm/event.c: 176 in hvm_event_breakpoint()
>> 170
>> 171     int hvm_event_breakpoint(unsigned long rip,
>> 172                              enum hvm_event_breakpoint_type type)
>> 173     {
>> 174         struct vcpu *curr = current;
>> 175         struct arch_domain *ad = &curr->domain->arch;
>>>>>      CID 1353192:  Uninitialized variables  (UNINIT)
>>>>>      Declaring variable "req" without initializer.
>> 176         vm_event_request_t req;
>> 177
>> 178         switch ( type )
>> 179         {
>> 180         case HVM_EVENT_SOFTWARE_BREAKPOINT:
>> 181             if ( !ad->monitor.software_breakpoint_enabled )
>>
>>
>> _________________________________________________________________________
>> _______________________________
>> To view the defects in Coverity Scan visit, https://scan.coverity.com/pro
>> jects/xenproject?tab=overview
>>
>> To manage Coverity Scan email notifications for
>> "ian.campbell@citrix.com", click https://scan.coverity.com/subscriptions/
>> edit?email=ian.campbell%40citrix.com&token=1ce0fc428b9f94f66fd8d1ecf6cbb7
>> 6a
>>

Sorry, my bad, I didn't know struct-initialization using labels sets all 
the other fields to zero.
Shall I submit a fix for this issue?

Corneliu.

Re: Uninitialized variables in hvm_event_breakpoint (Re: New Defects reported by Coverity Scan for XenProject)

[Razvan Cojocaru]

On 02/18/2016 12:01 PM, Ian Campbell wrote:
> On Wed, 2016-02-17 at 16:02 -0800, scan-admin@coverity.com wrote:
>> Hi,
>>
>> Please find the latest report on new defect(s) introduced to XenProject
>> found with Coverity Scan.
>>
>> 1 new defect(s) introduced to XenProject found with Coverity Scan.
>> 4 defect(s), reported by Coverity Scan earlier, were marked fixed in the
>> recent build analyzed by Coverity Scan.
>>
>> New defect(s) Reported-by: Coverity Scan
>> Showing 1 of 1 defect(s)
>>
>>
>> ** CID 1353192:  Uninitialized variables  (UNINIT)
>> /xen/arch/x86/hvm/event.c: 176 in hvm_event_breakpoint()
> 
> This appears to have been introduced by: 
>     commit
>     557c7873f35aa39bd84977b28948457b1b342f92
>     Author: Corneliu ZUZU <czuzu@bitdef
>     ender.com>
>     Date:   Mon Feb 15 14:14:16 2016 +0100
> 
>         x86: merge 2 hvm_event_... functions into 1
>         
>         This patch merges almost identical functions hvm_event_int3 and
>         hvm_event_single_step into a single function called hvm_event_breakpoint.
>         Also fixes event.c file header comment in the process.
>         
>         Signed-off-by: Corneliu ZUZU <    czuzu@bitdefender.com    >
>         Acked-by: Razvan Cojocaru <    rcojocaru@bitdefender.com    >
>         Acked-by: Jan Beulich <    jbeulich@suse.com    >
> 
> 
> hvm_event_breakpoint calls hvm_event_traps(&req) and if sync is true that
> ors some bits into req->flags which was never initialised.
> 
>>
>>
>> _________________________________________________________________________
>> _______________________________
>> *** CID 1353192:  Uninitialized variables  (UNINIT)
>> /xen/arch/x86/hvm/event.c: 176 in hvm_event_breakpoint()
>> 170     
>> 171     int hvm_event_breakpoint(unsigned long rip,
>> 172                              enum hvm_event_breakpoint_type type)
>> 173     {
>> 174         struct vcpu *curr = current;
>> 175         struct arch_domain *ad = &curr->domain->arch;
>>>>>     CID 1353192:  Uninitialized variables  (UNINIT)
>>>>>     Declaring variable "req" without initializer.
>> 176         vm_event_request_t req;
>> 177     
>> 178         switch ( type )
>> 179         {
>> 180         case HVM_EVENT_SOFTWARE_BREAKPOINT:
>> 181             if ( !ad->monitor.software_breakpoint_enabled )

But the structure is being initialized in both cases
(HVM_EVENT_SOFTWARE_BREAKPOINT and HVM_EVENT_SINGLESTEP_BREAKPOINT), and
the default case returns, so it's not possible to get to the
hvm_event_traps(&req) call with an uninitialized req. Am I missing
something?


Thanks,
Razvan

Re: Uninitialized variables in hvm_event_breakpoint (Re: New Defects reported by Coverity Scan for XenProject)

[Razvan Cojocaru]

On 02/18/2016 12:13 PM, Corneliu ZUZU wrote:
> On 2/18/2016 12:01 PM, Ian Campbell wrote:
>> On Wed, 2016-02-17 at 16:02 -0800, scan-admin@coverity.com wrote:
>>> Hi,
>>>
>>> Please find the latest report on new defect(s) introduced to XenProject
>>> found with Coverity Scan.
>>>
>>> 1 new defect(s) introduced to XenProject found with Coverity Scan.
>>> 4 defect(s), reported by Coverity Scan earlier, were marked fixed in the
>>> recent build analyzed by Coverity Scan.
>>>
>>> New defect(s) Reported-by: Coverity Scan
>>> Showing 1 of 1 defect(s)
>>>
>>>
>>> ** CID 1353192:  Uninitialized variables  (UNINIT)
>>> /xen/arch/x86/hvm/event.c: 176 in hvm_event_breakpoint()
>> This appears to have been introduced by:
>>      commit
>>      557c7873f35aa39bd84977b28948457b1b342f92
>>      Author: Corneliu ZUZU <czuzu@bitdef
>>      ender.com>
>>      Date:   Mon Feb 15 14:14:16 2016 +0100
>>
>>          x86: merge 2 hvm_event_... functions into 1
>>                   This patch merges almost identical functions
>> hvm_event_int3 and
>>          hvm_event_single_step into a single function called
>> hvm_event_breakpoint.
>>          Also fixes event.c file header comment in the process.
>>                   Signed-off-by: Corneliu ZUZU <   
>> czuzu@bitdefender.com    >
>>          Acked-by: Razvan Cojocaru <    rcojocaru@bitdefender.com    >
>>          Acked-by: Jan Beulich <    jbeulich@suse.com    >
>>
>>
>> hvm_event_breakpoint calls hvm_event_traps(&req) and if sync is true that
>> ors some bits into req->flags which was never initialised.
>>
>>>
>>> _________________________________________________________________________
>>>
>>> _______________________________
>>> *** CID 1353192:  Uninitialized variables  (UNINIT)
>>> /xen/arch/x86/hvm/event.c: 176 in hvm_event_breakpoint()
>>> 170
>>> 171     int hvm_event_breakpoint(unsigned long rip,
>>> 172                              enum hvm_event_breakpoint_type type)
>>> 173     {
>>> 174         struct vcpu *curr = current;
>>> 175         struct arch_domain *ad = &curr->domain->arch;
>>>>>>      CID 1353192:  Uninitialized variables  (UNINIT)
>>>>>>      Declaring variable "req" without initializer.
>>> 176         vm_event_request_t req;
>>> 177
>>> 178         switch ( type )
>>> 179         {
>>> 180         case HVM_EVENT_SOFTWARE_BREAKPOINT:
>>> 181             if ( !ad->monitor.software_breakpoint_enabled )
>>>
>>>
>>> _________________________________________________________________________
>>>
>>> _______________________________
>>> To view the defects in Coverity Scan visit,
>>> https://scan.coverity.com/pro
>>> jects/xenproject?tab=overview
>>>
>>> To manage Coverity Scan email notifications for
>>> "ian.campbell@citrix.com", click
>>> https://scan.coverity.com/subscriptions/
>>> edit?email=ian.campbell%40citrix.com&token=1ce0fc428b9f94f66fd8d1ecf6cbb7
>>>
>>> 6a
>>>
> 
> Sorry, my bad, I didn't know struct-initialization using labels sets all
> the other fields to zero.
> Shall I submit a fix for this issue?

Right, req->flags. Good catch. Yes, please do, req = {}; should do.


Thanks,
Razvan