Re: [ovs-dev] [PATCH 4/5] ofproto-dpif: Fix use of uninitialized attributes of timeout policy.

2021-05-24 Thread Ilya Maximets 
On 5/20/21 6:49 PM, Mark Gray wrote:
> On 04/04/2021 18:31, Ilya Maximets wrote:
>> 'cdtp' is allocated on a stack and it has uninitialized 'present'
>> field that specifies which attributes are actually set.  This
>> causes use of uninitialized attributes.
>>
>>  Conditional jump or move depends on uninitialised value(s)
>> at 0x539651: dpif_netlink_get_nl_tp_udp_attrs (dpif-netlink.c:3206)
>> by 0x539651: dpif_netlink_get_nl_tp_attrs (dpif-netlink.c:3234)
>> by 0x539651: dpif_netlink_ct_set_timeout_policy (dpif-netlink.c:3370)
>> by 0x42B615: ct_add_timeout_policy_to_dpif (ofproto-dpif.c:5421)
>> by 0x42B615: ct_set_zone_timeout_policy (ofproto-dpif.c:5525)
>> by 0x40BD98: ct_zones_reconfigure (bridge.c:756)
>> by 0x40BD98: datapath_reconfigure (bridge.c:804)
>> by 0x40D737: bridge_reconfigure (bridge.c:903)
>> by 0x411720: bridge_run (bridge.c:3331)
>> by 0x40751C: main (ovs-vswitchd.c:127)
>>
>> Clearing the whole structure to avoid this kind of problems.
>>
>> CC: Yi-Hung Wei 
>> Fixes: 993cae678bca ("ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy 
>> tables")
>> Signed-off-by: Ilya Maximets 
>> ---
>>  ofproto/ofproto-dpif.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index fd0b2fdea..47db9bb57 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -5413,6 +5413,8 @@ ct_add_timeout_policy_to_dpif(struct dpif *dpif,
>>  struct ct_dpif_timeout_policy cdtp;
>>  struct simap_node *node;
>>  
>> +memset(, 0, sizeof cdtp);
>> +
>>  cdtp.id = ct_tp->tp_id;
>>  SIMAP_FOR_EACH (node, _tp->tp) {
>>  ct_dpif_set_timeout_policy_attr_by_name(, node->name, 
>> node->data);
>>
> LGTM
> 
> Acked-by: Mark D. Gray 
> 

Thanks!  Applied to master and backported down to 2.13.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 4/5] ofproto-dpif: Fix use of uninitialized attributes of timeout policy.

2021-05-20 Thread Mark Gray 
On 04/04/2021 18:31, Ilya Maximets wrote:
> 'cdtp' is allocated on a stack and it has uninitialized 'present'
> field that specifies which attributes are actually set.  This
> causes use of uninitialized attributes.
> 
>  Conditional jump or move depends on uninitialised value(s)
> at 0x539651: dpif_netlink_get_nl_tp_udp_attrs (dpif-netlink.c:3206)
> by 0x539651: dpif_netlink_get_nl_tp_attrs (dpif-netlink.c:3234)
> by 0x539651: dpif_netlink_ct_set_timeout_policy (dpif-netlink.c:3370)
> by 0x42B615: ct_add_timeout_policy_to_dpif (ofproto-dpif.c:5421)
> by 0x42B615: ct_set_zone_timeout_policy (ofproto-dpif.c:5525)
> by 0x40BD98: ct_zones_reconfigure (bridge.c:756)
> by 0x40BD98: datapath_reconfigure (bridge.c:804)
> by 0x40D737: bridge_reconfigure (bridge.c:903)
> by 0x411720: bridge_run (bridge.c:3331)
> by 0x40751C: main (ovs-vswitchd.c:127)
> 
> Clearing the whole structure to avoid this kind of problems.
> 
> CC: Yi-Hung Wei 
> Fixes: 993cae678bca ("ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy 
> tables")
> Signed-off-by: Ilya Maximets 
> ---
>  ofproto/ofproto-dpif.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index fd0b2fdea..47db9bb57 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5413,6 +5413,8 @@ ct_add_timeout_policy_to_dpif(struct dpif *dpif,
>  struct ct_dpif_timeout_policy cdtp;
>  struct simap_node *node;
>  
> +memset(, 0, sizeof cdtp);
> +
>  cdtp.id = ct_tp->tp_id;
>  SIMAP_FOR_EACH (node, _tp->tp) {
>  ct_dpif_set_timeout_policy_attr_by_name(, node->name, 
> node->data);
> 
LGTM

Acked-by: Mark D. Gray 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 4/5] ofproto-dpif: Fix use of uninitialized attributes of timeout policy.

2021-04-04 Thread Ilya Maximets 
'cdtp' is allocated on a stack and it has uninitialized 'present'
field that specifies which attributes are actually set.  This
causes use of uninitialized attributes.

 Conditional jump or move depends on uninitialised value(s)
at 0x539651: dpif_netlink_get_nl_tp_udp_attrs (dpif-netlink.c:3206)
by 0x539651: dpif_netlink_get_nl_tp_attrs (dpif-netlink.c:3234)
by 0x539651: dpif_netlink_ct_set_timeout_policy (dpif-netlink.c:3370)
by 0x42B615: ct_add_timeout_policy_to_dpif (ofproto-dpif.c:5421)
by 0x42B615: ct_set_zone_timeout_policy (ofproto-dpif.c:5525)
by 0x40BD98: ct_zones_reconfigure (bridge.c:756)
by 0x40BD98: datapath_reconfigure (bridge.c:804)
by 0x40D737: bridge_reconfigure (bridge.c:903)
by 0x411720: bridge_run (bridge.c:3331)
by 0x40751C: main (ovs-vswitchd.c:127)

Clearing the whole structure to avoid this kind of problems.

CC: Yi-Hung Wei 
Fixes: 993cae678bca ("ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy 
tables")
Signed-off-by: Ilya Maximets 
---
 ofproto/ofproto-dpif.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index fd0b2fdea..47db9bb57 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5413,6 +5413,8 @@ ct_add_timeout_policy_to_dpif(struct dpif *dpif,
 struct ct_dpif_timeout_policy cdtp;
 struct simap_node *node;
 
+memset(, 0, sizeof cdtp);
+
 cdtp.id = ct_tp->tp_id;
 SIMAP_FOR_EACH (node, _tp->tp) {
 ct_dpif_set_timeout_policy_attr_by_name(, node->name, node->data);
-- 
2.26.2

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev