| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20210115171203.175115-1-alobakin@pm.me>
Date: Fri, 15 Jan 2021 17:12:33 +0000
From: Alexander Lobakin <alobakin@...me>
To: Dongseok Yi <dseok.yi@...sung.com>
Cc: Alexander Lobakin <alobakin@...me>,
"David S. Miller" <davem@...emloft.net>,
Steffen Klassert <steffen.klassert@...unet.com>,
namkyu78.kim@...sung.com, Jakub Kicinski <kuba@...nel.org>,
Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
Willem de Bruijn <willemb@...gle.com>,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2] udp: ipv4: manipulate network header of NATed UDP GRO fraglist
From: Dongseok Yi <dseok.yi@...sung.com>
Date: Fri, 15 Jan 2021 22:20:35 +0900
> UDP/IP header of UDP GROed frag_skbs are not updated even after NAT
> forwarding. Only the header of head_skb from ip_finish_output_gso ->
> skb_gso_segment is updated but following frag_skbs are not updated.
>
> A call path skb_mac_gso_segment -> inet_gso_segment ->
> udp4_ufo_fragment -> __udp_gso_segment -> __udp_gso_segment_list
> does not try to update UDP/IP header of the segment list but copy
> only the MAC header.
>
> Update dport, daddr and checksums of each skb of the segment list
> in __udp_gso_segment_list. It covers both SNAT and DNAT.
>
> Fixes: 9fd1ff5d2ac7 (udp: Support UDP fraglist GRO/GSO.)
> Signed-off-by: Dongseok Yi <dseok.yi@...sung.com>
> ---
> v1:
> Steffen Klassert said, there could be 2 options.
> https://lore.kernel.org/patchwork/patch/1362257/
> I was trying to write a quick fix, but it was not easy to forward
> segmented list. Currently, assuming DNAT only.
>
> v2:
> Per Steffen Klassert request, move the procedure from
> udp4_ufo_fragment to __udp_gso_segment_list and support SNAT.
>
> To Alexander Lobakin, I've checked your email late. Just use this
> patch as a reference. It support SNAT too, but does not support IPv6
> yet. I cannot make IPv6 header changes in __udp_gso_segment_list due
> to the file is in IPv4 directory.
I used another approach, tried to make fraglist GRO closer to plain
in terms of checksummming, as it is confusing to me why GSO packet
should have CHECKSUM_UNNECESSARY. Just let Netfilter do its mangling,
and then use classic UDP GSO magic at the end of segmentation.
I also see the idea of explicit comparing and editing of IP and UDP
headers right in __udp_gso_segment_list() rather unacceptable.
Dongseok, Steffen, please test this WIP diff and tell if this one
works for you, so I could clean up the code and make a patch.
For me, it works now in any configurations, with and without
checksum/GSO/fraglist offload.
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index c1a6f262636a..646a42e88e83 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3674,6 +3674,7 @@ struct sk_buff *skb_segment_list(struct sk_buff *skb,
unsigned int offset)
{
struct sk_buff *list_skb = skb_shinfo(skb)->frag_list;
+ unsigned int doffset = skb->data - skb_mac_header(skb);
unsigned int tnl_hlen = skb_tnl_header_len(skb);
unsigned int delta_truesize = 0;
unsigned int delta_len = 0;
@@ -3681,7 +3682,7 @@ struct sk_buff *skb_segment_list(struct sk_buff *skb,
struct sk_buff *nskb, *tmp;
int err;
- skb_push(skb, -skb_network_offset(skb) + offset);
+ skb_push(skb, doffset);
skb_shinfo(skb)->frag_list = NULL;
@@ -3716,12 +3717,11 @@ struct sk_buff *skb_segment_list(struct sk_buff *skb,
delta_len += nskb->len;
delta_truesize += nskb->truesize;
- skb_push(nskb, -skb_network_offset(nskb) + offset);
+ skb_push(nskb, skb_headroom(nskb) - skb_headroom(skb));
skb_release_head_state(nskb);
- __copy_skb_header(nskb, skb);
+ __copy_skb_header(nskb, skb);
- skb_headers_offset_update(nskb, skb_headroom(nskb) - skb_headroom(skb));
skb_copy_from_linear_data_offset(skb, -tnl_hlen,
nskb->data - tnl_hlen,
offset + tnl_hlen);
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index ff39e94781bf..61665fcd8c85 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -190,13 +190,58 @@ EXPORT_SYMBOL(skb_udp_tunnel_segment);
static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
netdev_features_t features)
{
- unsigned int mss = skb_shinfo(skb)->gso_size;
+ struct sk_buff *seg;
+ struct udphdr *uh;
+ unsigned int mss;
+ __be16 newlen;
+ __sum16 check;
+
+ mss = skb_shinfo(skb)->gso_size;
+ if (skb->len <= sizeof(*uh) + mss)
+ return ERR_PTR(-EINVAL);
- skb = skb_segment_list(skb, features, skb_mac_header_len(skb));
+ skb_pull(skb, sizeof(*uh));
+
+ skb = skb_segment_list(skb, features, skb->data - skb_mac_header(skb));
if (IS_ERR(skb))
return skb;
- udp_hdr(skb)->len = htons(sizeof(struct udphdr) + mss);
+ seg = skb;
+ uh = udp_hdr(seg);
+
+ /* compute checksum adjustment based on old length versus new */
+ newlen = htons(sizeof(*uh) + mss);
+ check = csum16_add(csum16_sub(uh->check, uh->len), newlen);
+
+ for (;;) {
+ if (!seg->next)
+ break;
+
+ uh->len = newlen;
+ uh->check = check;
+
+ if (seg->ip_summed == CHECKSUM_PARTIAL)
+ gso_reset_checksum(seg, ~check);
+ else
+ uh->check = gso_make_checksum(seg, ~check) ? :
+ CSUM_MANGLED_0;
+
+ seg = seg->next;
+ uh = udp_hdr(seg);
+ }
+
+ /* last packet can be partial gso_size, account for that in checksum */
+ newlen = htons(skb_tail_pointer(seg) - skb_transport_header(seg) +
+ seg->data_len);
+ check = csum16_add(csum16_sub(uh->check, uh->len), newlen);
+
+ uh->len = newlen;
+ uh->check = check;
+
+ if (seg->ip_summed == CHECKSUM_PARTIAL)
+ gso_reset_checksum(seg, ~check);
+ else
+ uh->check = gso_make_checksum(seg, ~check) ? : CSUM_MANGLED_0;
return skb;
}
@@ -602,27 +647,13 @@ INDIRECT_CALLABLE_SCOPE int udp4_gro_complete(struct sk_buff *skb, int nhoff)
const struct iphdr *iph = ip_hdr(skb);
struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
- if (NAPI_GRO_CB(skb)->is_flist) {
- uh->len = htons(skb->len - nhoff);
-
- skb_shinfo(skb)->gso_type |= (SKB_GSO_FRAGLIST|SKB_GSO_UDP_L4);
- skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count;
-
- if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
- if (skb->csum_level < SKB_MAX_CSUM_LEVEL)
- skb->csum_level++;
- } else {
- skb->ip_summed = CHECKSUM_UNNECESSARY;
- skb->csum_level = 0;
- }
-
- return 0;
- }
-
if (uh->check)
uh->check = ~udp_v4_check(skb->len - nhoff, iph->saddr,
iph->daddr, 0);
+ if (NAPI_GRO_CB(skb)->is_flist)
+ skb_shinfo(skb)->gso_type |= SKB_GSO_FRAGLIST;
+
return udp_gro_complete(skb, nhoff, udp4_lib_lookup_skb);
}
Powered by blists - more mailing lists