From: Mikhail Dmitrichenko <[email protected]>

Software segmentation reuses the original packet as the first segment
and trims it with dp_packet_set_size().  Any L2 padding follows the TCP
payload, so it is cut off by the trim, but 'l2_pad_size' is left as is.

dp_packet_l3_size(), dp_packet_l4_size() and their inner variants
subtract 'l2_pad_size' from the packet size, so for the first segment
they under-report the actual length by the size of the former padding.
As a result, dp_packet_gso_update_segment() writes a wrong IPv4 total
length or IPv6 payload length, and the TCP checksum computed later in
software does not cover the whole segment.

If the padding is longer than the TCP header and 'tso_segsz' together,
these sizes wrap around.  The IPv6 payload length becomes bogus and
packet_tcp_complete_csum() reads past the end of the packet buffer:
up to 64 KB for IPv6 and until it faults for IPv4, which crashes
ovs-vswitchd:

  SIGSEGV detected, backtrace:
  csum_continue at lib/csum.c:46
  packet_tcp_complete_csum at lib/packets.c:2120
  dp_packet_ol_send_prepare at lib/dp-packet.c:601
  netdev_send at lib/netdev.c:845
  dp_netdev_pmd_flush_output_on_port at lib/dpif-netdev.c:4402

Such a packet can be received on a port with userspace TSO enabled when
the IP length is shorter than the frame, e.g. from a VM, and be sent to
a port without TSO support.

Reset 'l2_pad_size' when trimming the first segment.  The other segments
are freshly allocated and never carry padding.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: ef762327f6d3 ("dp-packet-gso: Refactor software segmentation code.")
Signed-off-by: Mikhail Dmitrichenko <[email protected]>
---
 lib/dp-packet-gso.c  | 14 ++++++++--
 tests/dpif-netdev.at | 61 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c
index 8c8817352..b89b54078 100644
--- a/lib/dp-packet-gso.c
+++ b/lib/dp-packet-gso.c
@@ -179,6 +179,15 @@ dp_packet_gso_update_segment(struct dp_packet *seg, 
unsigned int seg_no,
     }
 }
 
+/* Trims the first segment 'p' to 'size' bytes.  The L2 padding, if any,
+ * follows the TCP payload and is cut off along with it. */
+static void
+dp_packet_gso_trim_first_seg(struct dp_packet *p, size_t size)
+{
+    dp_packet_set_size(p, size);
+    dp_packet_set_l2_pad_size(p, 0);
+}
+
 static void
 dp_packet_gso__(struct dp_packet *p, struct dp_packet_batch *batch,
                 bool partial_seg)
@@ -241,7 +250,8 @@ last_seg:
 first_seg:
     if (partial_seg) {
         if (dp_packet_gso_partial_nr_segs(p) != 1) {
-            dp_packet_set_size(p, hdr_len + (n_segs - 1) * tso_segsz);
+            dp_packet_gso_trim_first_seg(p,
+                                         hdr_len + (n_segs - 1) * tso_segsz);
             if (n_segs == 2) {
                 /* No need to ask HW segmentation, we already did the job. */
                 dp_packet_set_tso_segsz(p, 0);
@@ -249,7 +259,7 @@ first_seg:
         }
     } else {
         /* Trim the first segment and reset TSO. */
-        dp_packet_set_size(p, hdr_len + tso_segsz);
+        dp_packet_gso_trim_first_seg(p, hdr_len + tso_segsz);
         dp_packet_set_tso_segsz(p, 0);
     }
     dp_packet_gso_update_segment(p, 0, n_segs, tso_segsz, udp_tnl, gre_tnl);
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 14f238e62..cc4b1d57a 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -3333,6 +3333,67 @@ AT_CHECK_UNQUOTED([ovs-pcap p2.pcap], [0], [dnl
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([dpif-netdev - tso + l2 padding])
+AT_KEYWORDS([userspace offload])
+OVS_VSWITCHD_START(
+  [set Open_vSwitch . other_config:userspace-tso-enable=true -- \
+   add-br br1 -- set bridge br1 datapath-type=dummy -- \
+   add-port br1 p1 -- \
+       set Interface p1 type=dummy -- \
+   add-port br1 p2 -- \
+       set Interface p2 type=dummy])
+
+AT_CHECK([ovs-ofctl add-flow br1 in_port=p1,actions=output:p2])
+
+AT_CHECK([ovs-vsctl set Interface p2 options:tx_pcap=p2.pcap -- \
+                    set Interface p1 options:ol_tso_segsz=500])
+
+dnl IPv4/TCP and IPv6/TCP packets with 1000 bytes of payload, to be
+dnl followed by L2 padding.
+zero500=$(printf '%0*d' 1000 0)
+pkt4="0a8f394fe0738abf7e2f0584080045010410000000004006000"
+pkt4="${pkt4}0c0a87b02c0a87b01d47814510000000000000000501000000000"
+pkt4="${pkt4}0000${zero500}${zero500}"
+pkt6="0a8f394fe0738abf7e2f058486dd6000000003fc06002001cafe00000000000000"
+pkt6="${pkt6}00000000882001cafe000000000000000000000092d47814510000000000000000"
+pkt6="${pkt6}5010000000000000${zero500}${zero500}"
+
+dnl Padding shorter and longer than the first segment's TCP header and
+dnl payload.
+pad_short=$(printf '%0*d' 20 0)
+pad_long=$(printf '%0*d' 1200 0)
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 "${pkt4}${pad_short}"])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 "${pkt4}${pad_long}"])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 "${pkt6}${pad_short}"])
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 "${pkt6}${pad_long}"])
+
+dnl Padding must be dropped: 2x 500 byte payloads per packet.
+AT_CHECK_UNQUOTED([ovs-pcap p2.pcap], [0], [dnl
+[0a8f394fe0738abf7e2f058408004501021c0000000040060187c0a87b02c0a87b01]dnl
+[d47814510000000000000000501000004dc20000${zero500}]
+[0a8f394fe0738abf7e2f058408004501021c0001000040060186c0a87b02c0a87b01]dnl
+[d4781451000001f400000000501000004bce0000${zero500}]
+[0a8f394fe0738abf7e2f058408004501021c0000000040060187c0a87b02c0a87b01]dnl
+[d47814510000000000000000501000004dc20000${zero500}]
+[0a8f394fe0738abf7e2f058408004501021c0001000040060186c0a87b02c0a87b01]dnl
+[d4781451000001f400000000501000004bce0000${zero500}]
+[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl
+[882001cafe000000000000000000000092d4781451000000000000000050100000edfd0000]dnl
+[${zero500}]
+[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl
+[882001cafe000000000000000000000092d4781451000001f40000000050100000ec090000]dnl
+[${zero500}]
+[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl
+[882001cafe000000000000000000000092d4781451000000000000000050100000edfd0000]dnl
+[${zero500}]
+[0a8f394fe0738abf7e2f058486dd60000000020806002001cafe0000000000000000000000]dnl
+[882001cafe000000000000000000000092d4781451000001f40000000050100000ec090000]dnl
+[${zero500}]
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([dpif-netdev - tunnel tso fallback])
 AT_KEYWORDS([userspace offload])
 OVS_VSWITCHD_START([set Open_vSwitch . other_config:userspace-tso-enable=true \
-- 
2.25.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to