diff mbox series

[ovs-dev,v2] dpif-netlink: fix using uninitialized info.tc_modify_flow_deleted in out label

Message ID 1617888792-25072-1-git-send-email-wangyunjian@huawei.com
State Accepted
Headers show
Series [ovs-dev,v2] dpif-netlink: fix using uninitialized info.tc_modify_flow_deleted in out label | expand

Commit Message

Yunjian Wang April 8, 2021, 1:33 p.m. UTC 
From: Yunjian Wang <wangyunjian@huawei.com>

Before info.tc_modify_flow_deleted is assigned a value, error
processing of other statements goes to the out label. In the
out label, the uninitialized variant is used for condition
determination, which may cause uncertain behavior.

Fixes: 65b84d4a32bd ("dpif-netlink: avoid netlink modify flow put op failed after tc modify flow put op failed.")
Signed-off-by: Mengfan Lv <lvmengfan@huawei.com>
Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
v2:
   update commit log
---
 lib/dpif-netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Simon Horman April 9, 2021, 11:28 a.m. UTC | #1 
On Thu, Apr 08, 2021 at 09:33:12PM +0800, wangyunjian wrote:
> From: Yunjian Wang <wangyunjian@huawei.com>
> 
> Before info.tc_modify_flow_deleted is assigned a value, error
> processing of other statements goes to the out label. In the
> out label, the uninitialized variant is used for condition
> determination, which may cause uncertain behavior.
> 
> Fixes: 65b84d4a32bd ("dpif-netlink: avoid netlink modify flow put op failed after tc modify flow put op failed.")
> Signed-off-by: Mengfan Lv <lvmengfan@huawei.com>
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>

Thanks, I see the problem you describe and agree
it is wise to take the approach of initialising this as early as possible.

Reviewed-by: Simon Horman <simon.horman@netronome.com>
Ilya Maximets April 20, 2021, 10:13 a.m. UTC | #2 
On 4/9/21 1:28 PM, Simon Horman wrote:
> On Thu, Apr 08, 2021 at 09:33:12PM +0800, wangyunjian wrote:
>> From: Yunjian Wang <wangyunjian@huawei.com>
>>
>> Before info.tc_modify_flow_deleted is assigned a value, error
>> processing of other statements goes to the out label. In the
>> out label, the uninitialized variant is used for condition
>> determination, which may cause uncertain behavior.
>>
>> Fixes: 65b84d4a32bd ("dpif-netlink: avoid netlink modify flow put op failed after tc modify flow put op failed.")
>> Signed-off-by: Mengfan Lv <lvmengfan@huawei.com>
>> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> 
> Thanks, I see the problem you describe and agree
> it is wise to take the approach of initialising this as early as possible.
> 
> Reviewed-by: Simon Horman <simon.horman@netronome.com>

Thanks!  Applied to master and backported down to 2.13.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index ceb56c685..50520f8c0 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2061,6 +2061,7 @@  parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put)
     uint8_t csum_on = false;
     int err;
 
+    info.tc_modify_flow_deleted = false;
     if (put->flags & DPIF_FP_PROBE) {
         return EOPNOTSUPP;
     }
@@ -2105,7 +2106,6 @@  parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put)
     info.tunnel_csum_on = csum_on;
     info.recirc_id_shared_with_tc = (dpif->user_features
                                      & OVS_DP_F_TC_RECIRC_SHARING);
-    info.tc_modify_flow_deleted = false;
     err = netdev_flow_put(dev, &match,
                           CONST_CAST(struct nlattr *, put->actions),
                           put->actions_len,