tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

ovpn: tcp - fix packet extraction from stream

When processing TCP stream data in ovpn_tcp_recv, we receive large
cloned skbs from __strp_rcv that may contain multiple coalesced packets.
The current implementation has two bugs:

1. Header offset overflow: Using pskb_pull with large offsets on
coalesced skbs causes skb->data - skb->head to exceed the u16 storage
of skb->network_header. This causes skb_reset_network_header to fail
on the inner decapsulated packet, resulting in packet drops.

2. Unaligned protocol headers: Extracting packets from arbitrary
positions within the coalesced TCP stream provides no alignment
guarantees for the packet data causing performance penalties on
architectures without efficient unaligned access. Additionally,
openvpn's 2-byte length prefix on TCP packets causes the subsequent
4-byte opcode and packet ID fields to be inherently misaligned.

Fix both issues by allocating a new skb for each openvpn packet and
using skb_copy_bits to extract only the packet content into the new
buffer, skipping the 2-byte length prefix. Also, check the length before
invoking the function that performs the allocation to avoid creating an
invalid skb.

If the packet has to be forwarded to userspace the 2-byte prefix can be
pushed to the head safely, without misalignment.

As a side effect, this approach also avoids the expensive linearization
that pskb_pull triggers on cloned skbs with page fragments. In testing,
this resulted in TCP throughput improvements of up to 74%.

Fixes: 11851cbd60ea ("ovpn: implement TCP transport")
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Ralf Lici and committed by
David S. Miller d4f687fb 3f1af485

+38 -19
+38 -19
drivers/net/ovpn/tcp.c
··· 70 70 peer->tcp.sk_cb.sk_data_ready(sk); 71 71 } 72 72 73 + static struct sk_buff *ovpn_tcp_skb_packet(const struct ovpn_peer *peer, 74 + struct sk_buff *orig_skb, 75 + const int pkt_len, const int pkt_off) 76 + { 77 + struct sk_buff *ovpn_skb; 78 + int err; 79 + 80 + /* create a new skb with only the content of the current packet */ 81 + ovpn_skb = netdev_alloc_skb(peer->ovpn->dev, pkt_len); 82 + if (unlikely(!ovpn_skb)) 83 + goto err; 84 + 85 + skb_copy_header(ovpn_skb, orig_skb); 86 + err = skb_copy_bits(orig_skb, pkt_off, skb_put(ovpn_skb, pkt_len), 87 + pkt_len); 88 + if (unlikely(err)) { 89 + net_warn_ratelimited("%s: skb_copy_bits failed for peer %u\n", 90 + netdev_name(peer->ovpn->dev), peer->id); 91 + kfree_skb(ovpn_skb); 92 + goto err; 93 + } 94 + 95 + consume_skb(orig_skb); 96 + return ovpn_skb; 97 + err: 98 + kfree_skb(orig_skb); 99 + return NULL; 100 + } 101 + 73 102 static void ovpn_tcp_rcv(struct strparser *strp, struct sk_buff *skb) 74 103 { 75 104 struct ovpn_peer *peer = container_of(strp, struct ovpn_peer, tcp.strp); 76 105 struct strp_msg *msg = strp_msg(skb); 77 - size_t pkt_len = msg->full_len - 2; 78 - size_t off = msg->offset + 2; 106 + int pkt_len = msg->full_len - 2; 79 107 u8 opcode; 80 108 81 - /* ensure skb->data points to the beginning of the openvpn packet */ 82 - if (!pskb_pull(skb, off)) { 83 - net_warn_ratelimited("%s: packet too small for peer %u\n", 84 - netdev_name(peer->ovpn->dev), peer->id); 85 - goto err; 86 - } 87 - 88 - /* strparser does not trim the skb for us, therefore we do it now */ 89 - if (pskb_trim(skb, pkt_len) != 0) { 90 - net_warn_ratelimited("%s: trimming skb failed for peer %u\n", 91 - netdev_name(peer->ovpn->dev), peer->id); 92 - goto err; 93 - } 94 - 95 - /* we need the first 4 bytes of data to be accessible 109 + /* we need at least 4 bytes of data in the packet 96 110 * to extract the opcode and the key ID later on 97 111 */ 98 - if (!pskb_may_pull(skb, OVPN_OPCODE_SIZE)) { 112 + if (unlikely(pkt_len < OVPN_OPCODE_SIZE)) { 99 113 net_warn_ratelimited("%s: packet too small to fetch opcode for peer %u\n", 100 114 netdev_name(peer->ovpn->dev), peer->id); 101 115 goto err; 102 116 } 117 + 118 + /* extract the packet into a new skb */ 119 + skb = ovpn_tcp_skb_packet(peer, skb, pkt_len, msg->offset + 2); 120 + if (unlikely(!skb)) 121 + goto err; 103 122 104 123 /* DATA_V2 packets are handled in kernel, the rest goes to user space */ 105 124 opcode = ovpn_opcode_from_skb(skb, 0); ··· 132 113 /* The packet size header must be there when sending the packet 133 114 * to userspace, therefore we put it back 134 115 */ 135 - skb_push(skb, 2); 116 + *(__be16 *)__skb_push(skb, sizeof(u16)) = htons(pkt_len); 136 117 ovpn_tcp_to_userspace(peer, strp->sk, skb); 137 118 return; 138 119 }