ofproto-dpif-xlate: Fix NULL pointer dereference in xlate_normal().

Considering the following flows:

ovs-ofctl dump-flows br0
 cookie=0x0, table=0, priority=0 actions=NORMAL

and assuming a packet originated from packet-out in this way:

ovs-ofctl packet-out br0 \
    "in_port=controller,packet=<UDP packet>,action=ct(table=0)"

If in_port is OFPP_NONE or OFPP_CONTROLLER, this leads to a
NULL pointer (xport) dereference in xlate_normal().

Fix it by checking the xport pointer validity while deciding whether
it is a candidate for mac learning or not.

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

2 files changed, 31 insertions, 2 deletions

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 2ec1f4eb8..f26938442 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3025,7 +3025,7 @@ xlate_normal(struct xlate_ctx *ctx)
     bool is_grat_arp = is_gratuitous_arp(flow, wc);
     if (ctx->xin->allow_side_effects
         && flow->packet_type == htonl(PT_ETH)
-        && in_port->pt_mode != NETDEV_PT_LEGACY_L3
+        && in_port && in_port->pt_mode != NETDEV_PT_LEGACY_L3
     ) {
         update_learning_table(ctx, in_xbundle, flow->dl_src, vlan,
                               is_grat_arp);
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 70a4f6b8a..48bb50f25 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5245,7 +5245,36 @@ check_flows () {
     echo "n_packets=$n"
     test "$n" = 1
 }
-OVS_WAIT_UNTIL([check_flows], [ovs dump-flows br0])
+OVS_WAIT_UNTIL([check_flows], [ovs-ofctl dump-flows br0])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+# Checks for regression against a bug in which OVS crashed
+# with in_port=OFPP_NONE or in_port=OFPP_CONTROLLER and
+# recirculation is involved.
+AT_SETUP([ofproto-dpif - packet-out recirculation with OFPP_NONE and OFPP_CONTROLLER])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2
+
+AT_DATA([flows.txt], [dnl
+table=0 ip actions=mod_dl_dst:83:83:83:83:83:83,ct(table=1)
+table=1 ip actions=ct(commit),normal
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+packet=ffffffffffff00102030405008004500001c00000000401100000a000002ffffffff0035111100080000
+AT_CHECK([ovs-ofctl packet-out br0 "in_port=none,packet=$packet actions=table"])
+AT_CHECK([ovs-ofctl packet-out br0 "in_port=controller,packet=$packet actions=table"])
+
+# Dumps out the flow table, extracts the number of packets that have gone
+# through the (single) flow in table 1, and returns success if it's exactly 2.
+check_flows () {
+    n=$(ovs-ofctl dump-flows br0 table=1 | sed -n 's/.*n_packets=\([[0-9]]\{1,\}\).*/\1/p')
+    echo "n_packets=$n"
+    test "$n" = 2
+}
+OVS_WAIT_UNTIL([check_flows], [ovs-ofctl dump-flows br0])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP