From patchwork Fri Jan 3 23:50:34 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1217464 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=fraxinus.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 47qM9h1JMPz9sPn for ; Sat, 4 Jan 2020 10:51:03 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 2915885FB6; Fri, 3 Jan 2020 23:51:02 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Lg8T-yy5FAJe; Fri, 3 Jan 2020 23:51:00 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id E827085F9A; Fri, 3 Jan 2020 23:51:00 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C9B80C1D80; Fri, 3 Jan 2020 23:51:00 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 22C7DC077D for ; Fri, 3 Jan 2020 23:50:59 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 0ADAE20355 for ; Fri, 3 Jan 2020 23:50:59 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qfbyuA7qPvPo for ; Fri, 3 Jan 2020 23:50:56 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by silver.osuosl.org (Postfix) with ESMTPS id CED4320115 for ; Fri, 3 Jan 2020 23:50:55 +0000 (UTC) X-Originating-IP: 90.177.210.238 Received: from localhost.localdomain (238.210.broadband10.iol.cz [90.177.210.238]) (Authenticated sender: i.maximets@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id A2A0B1C0002; Fri, 3 Jan 2020 23:50:52 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org, Ben Pfaff Date: Sat, 4 Jan 2020 00:50:34 +0100 Message-Id: <20200103235034.28656-1-i.maximets@ovn.org> X-Mailer: git-send-email 2.17.1 Cc: Ilya Maximets Subject: [ovs-dev] [PATCH] ofproto-dpif: Fix using uninitialized execute hash. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Most of callers doesn't initialize dpif_execute.hash leaving random value from the stack. And this random value used later while encoding netlink message and might produce unwanted kernel behavior. Fix that by fully initializing dpif_execute structure. Using designated initializers to avoid such issues in the future. Fixes: 0442bfb11d6c ("ofproto-dpif-upcall: Echo HASH attribute back to datapath.") Signed-off-by: Ilya Maximets Acked-by: William Tu Acked-by: Ben Pfaff --- ofproto/ofproto-dpif.c | 113 ++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 63 deletions(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index b17c6cdca..d298e17cf 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -1073,7 +1073,6 @@ check_masked_set_action(struct dpif_backer *backer) { struct eth_header *eth; struct ofpbuf actions; - struct dpif_execute execute; struct dp_packet packet; struct flow flow; int error; @@ -1098,14 +1097,13 @@ check_masked_set_action(struct dpif_backer *backer) /* Execute the actions. On older datapaths this fails with EINVAL, on * newer datapaths it succeeds. */ - execute.actions = actions.data; - execute.actions_len = actions.size; - execute.packet = &packet; - execute.flow = &flow; - execute.needs_help = false; - execute.probe = true; - execute.mtu = 0; - + struct dpif_execute execute = { + .actions = actions.data, + .actions_len = actions.size, + .packet = &packet, + .flow = &flow, + .probe = true, + }; error = dpif_execute(backer->dpif, &execute); dp_packet_uninit(&packet); @@ -1127,7 +1125,6 @@ check_trunc_action(struct dpif_backer *backer) { struct eth_header *eth; struct ofpbuf actions; - struct dpif_execute execute; struct dp_packet packet; struct ovs_action_trunc *trunc; struct flow flow; @@ -1152,14 +1149,13 @@ check_trunc_action(struct dpif_backer *backer) /* Execute the actions. On older datapaths this fails with EINVAL, on * newer datapaths it succeeds. */ - execute.actions = actions.data; - execute.actions_len = actions.size; - execute.packet = &packet; - execute.flow = &flow; - execute.needs_help = false; - execute.probe = true; - execute.mtu = 0; - + struct dpif_execute execute = { + .actions = actions.data, + .actions_len = actions.size, + .packet = &packet, + .flow = &flow, + .probe = true, + }; error = dpif_execute(backer->dpif, &execute); dp_packet_uninit(&packet); @@ -1181,7 +1177,6 @@ check_trunc_action(struct dpif_backer *backer) static bool check_clone(struct dpif_backer *backer) { - struct dpif_execute execute; struct eth_header *eth; struct flow flow; struct dp_packet packet; @@ -1204,14 +1199,13 @@ check_clone(struct dpif_backer *backer) /* Execute the actions. On older datapaths this fails with EINVAL, on * newer datapaths it succeeds. */ - execute.actions = actions.data; - execute.actions_len = actions.size; - execute.packet = &packet; - execute.flow = &flow; - execute.needs_help = false; - execute.probe = true; - execute.mtu = 0; - + struct dpif_execute execute = { + .actions = actions.data, + .actions_len = actions.size, + .packet = &packet, + .flow = &flow, + .probe = true, + }; error = dpif_execute(backer->dpif, &execute); dp_packet_uninit(&packet); @@ -1233,7 +1227,6 @@ check_clone(struct dpif_backer *backer) static bool check_ct_eventmask(struct dpif_backer *backer) { - struct dpif_execute execute; struct dp_packet packet; struct ofpbuf actions; struct flow flow = { @@ -1266,14 +1259,13 @@ check_ct_eventmask(struct dpif_backer *backer) /* Execute the actions. On older datapaths this fails with EINVAL, on * newer datapaths it succeeds. */ - execute.actions = actions.data; - execute.actions_len = actions.size; - execute.packet = &packet; - execute.flow = &flow; - execute.needs_help = false; - execute.probe = true; - execute.mtu = 0; - + struct dpif_execute execute = { + .actions = actions.data, + .actions_len = actions.size, + .packet = &packet, + .flow = &flow, + .probe = true, + }; error = dpif_execute(backer->dpif, &execute); dp_packet_uninit(&packet); @@ -1328,7 +1320,6 @@ check_ct_clear(struct dpif_backer *backer) static bool check_ct_timeout_policy(struct dpif_backer *backer) { - struct dpif_execute execute; struct dp_packet packet; struct ofpbuf actions; struct flow flow = { @@ -1361,14 +1352,13 @@ check_ct_timeout_policy(struct dpif_backer *backer) /* Execute the actions. On older datapaths this fails with EINVAL, on * newer datapaths it succeeds. */ - execute.actions = actions.data; - execute.actions_len = actions.size; - execute.packet = &packet; - execute.flow = &flow; - execute.needs_help = false; - execute.probe = true; - execute.mtu = 0; - + struct dpif_execute execute = { + .actions = actions.data, + .actions_len = actions.size, + .packet = &packet, + .flow = &flow, + .probe = true, + }; error = dpif_execute(backer->dpif, &execute); dp_packet_uninit(&packet); @@ -1480,7 +1470,6 @@ check_nd_extensions(struct dpif_backer *backer) { struct eth_header *eth; struct ofpbuf actions; - struct dpif_execute execute; struct dp_packet packet; struct flow flow; int error; @@ -1500,14 +1489,13 @@ check_nd_extensions(struct dpif_backer *backer) flow_extract(&packet, &flow); /* Execute the actions. On datapaths without support fails with EINVAL. */ - execute.actions = actions.data; - execute.actions_len = actions.size; - execute.packet = &packet; - execute.flow = &flow; - execute.needs_help = false; - execute.probe = true; - execute.mtu = 0; - + struct dpif_execute execute = { + .actions = actions.data, + .actions_len = actions.size, + .packet = &packet, + .flow = &flow, + .probe = true, + }; error = dpif_execute(backer->dpif, &execute); dp_packet_uninit(&packet); @@ -4160,7 +4148,6 @@ ofproto_dpif_execute_actions__(struct ofproto_dpif *ofproto, struct dpif_flow_stats stats; struct xlate_out xout; struct xlate_in xin; - struct dpif_execute execute; int error; ovs_assert((rule != NULL) != (ofpacts != NULL)); @@ -4185,15 +4172,15 @@ ofproto_dpif_execute_actions__(struct ofproto_dpif *ofproto, goto out; } - execute.actions = odp_actions.data; - execute.actions_len = odp_actions.size; - pkt_metadata_from_flow(&packet->md, flow); - execute.packet = packet; - execute.flow = flow; - execute.needs_help = (xout.slow & SLOW_ACTION) != 0; - execute.probe = false; - execute.mtu = 0; + + struct dpif_execute execute = { + .actions = odp_actions.data, + .actions_len = odp_actions.size, + .packet = packet, + .flow = flow, + .needs_help = (xout.slow & SLOW_ACTION) != 0, + }; /* Fix up in_port. */ ofproto_dpif_set_packet_odp_port(ofproto, flow->in_port.ofp_port, packet);