| Message ID | 164703112546.2423483.15628969556445986960.stgit@fed.void |
|---|---|
| State | Accepted |
| Commit | 0027b3b46c759b65df9533b52166da7bbf3b96a1 |
| Headers | show |
| Series | [ovs-dev] ofproto-dpif-xlate: Fix NULL pointer dereference in xlate_normal() | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/intel-ovs-compilation | success | test: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 11 Mar 2022, at 21:38, Paolo Valerio wrote: > Considering the following flows: > > ovs-ofctl dump-flows br0 > cookie=0x0, duration=2431.944s, table=0, n_packets=0, n_bytes=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=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000, 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. > > Signed-off-by: Paolo Valerio <pvalerio@redhat.com> > --- Reviewed and tested, and all looks fine. Acked-by: Eelco Chaudron <echaudro@redhat.com>
On 3/14/22 09:51, Eelco Chaudron wrote: > > > On 11 Mar 2022, at 21:38, Paolo Valerio wrote: > >> Considering the following flows: >> >> ovs-ofctl dump-flows br0 >> cookie=0x0, duration=2431.944s, table=0, n_packets=0, n_bytes=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=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000, 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. >> >> Signed-off-by: Paolo Valerio <pvalerio@redhat.com> >> --- > > Reviewed and tested, and all looks fine. > > Acked-by: Eelco Chaudron <echaudro@redhat.com> Thanks! Applied. Best regards, Ilya Maximets.
On 4/5/22 00:45, Ilya Maximets wrote: > On 3/14/22 09:51, Eelco Chaudron wrote: >> >> >> On 11 Mar 2022, at 21:38, Paolo Valerio wrote: >> >>> Considering the following flows: >>> >>> ovs-ofctl dump-flows br0 >>> cookie=0x0, duration=2431.944s, table=0, n_packets=0, n_bytes=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=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000, 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. >>> >>> Signed-off-by: Paolo Valerio <pvalerio@redhat.com> >>> --- >> >> Reviewed and tested, and all looks fine. >> >> Acked-by: Eelco Chaudron <echaudro@redhat.com> > > Thanks! Applied. And backported down to 2.13. > > Best regards, Ilya Maximets.
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index cc9c1c628..d3b9648c2 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3032,7 +3032,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 912f3a1da..6a2bce336 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -5578,6 +5578,35 @@ OVS_WAIT_UNTIL([check_flows], [ovs 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 dump-flows br0]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto-dpif - debug_slow action]) OVS_VSWITCHD_START add_of_ports br0 1 2 3
Considering the following flows: ovs-ofctl dump-flows br0 cookie=0x0, duration=2431.944s, table=0, n_packets=0, n_bytes=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=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000, 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. Signed-off-by: Paolo Valerio <pvalerio@redhat.com> --- ofproto/ofproto-dpif-xlate.c | 2 +- tests/ofproto-dpif.at | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-)