diff mbox series

[v2] net: ipv6: fix wrong start position when receive hop-by-hop fragment

Message ID 20240422091917.3221601-1-gaoxingwang1@huawei.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [v2] net: ipv6: fix wrong start position when receive hop-by-hop fragment | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 926 this patch: 926
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 2 blamed authors not CCed: pablo@netfilter.org geokohma@cisco.com; 2 maintainers not CCed: pablo@netfilter.org geokohma@cisco.com
netdev/build_clang success Errors and warnings before: 937 this patch: 937
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 937 this patch: 937
netdev/checkpatch warning WARNING: From:/Signed-off-by: email name mismatch: 'From: gaoxingwang <gaoxingwang1@huawei.com>' != 'Signed-off-by: Gao Xingwang <gaoxingwang1@huawei.com>' WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: 9d9e937b1c8b ("ipv6/netfilter: Discard first fragment not including all headers")' WARNING: line length of 103 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-04-24--15-00 (tests: 995)

Commit Message

gaoxingwang April 22, 2024, 9:19 a.m. UTC
In IPv6, ipv6_rcv_core will parse the hop-by-hop type extension header and increase skb->transport_header by one extension header length.
But if there are more other extension headers like fragment header at this time, the skb->transport_header points to the second extension header,
not the transport layer header or the first extension header.

This will result in the start and nexthdrp variable not pointing to the same position in ipv6frag_thdr_trunced,
and ipv6_skip_exthdr returning incorrect offset and frag_off.Sometimes,the length of the last sharded packet is smaller than the calculated incorrect offset, resulting in packet loss.
We can use network header to offset and calculate the correct position to solve this problem.

Fixes: 9d9e937b1c8b (ipv6/netfilter: Discard first fragment not including all headers)
Signed-off-by: Gao Xingwang <gaoxingwang1@huawei.com>
---
 net/ipv6/reassembly.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jakub Kicinski April 25, 2024, 2:51 a.m. UTC | #1
On Mon, 22 Apr 2024 17:19:17 +0800 gaoxingwang wrote:
> In IPv6, ipv6_rcv_core will parse the hop-by-hop type extension header and increase skb->transport_header by one extension header length.
> But if there are more other extension headers like fragment header at this time, the skb->transport_header points to the second extension header,
> not the transport layer header or the first extension header.
> 
> This will result in the start and nexthdrp variable not pointing to the same position in ipv6frag_thdr_trunced,
> and ipv6_skip_exthdr returning incorrect offset and frag_off.Sometimes,the length of the last sharded packet is smaller than the calculated incorrect offset, resulting in packet loss.
> We can use network header to offset and calculate the correct position to solve this problem.
> 
> Fixes: 9d9e937b1c8b (ipv6/netfilter: Discard first fragment not including all headers)

nits:
 - this is not correct format for Fixes, missing quotes
 - please wrap the message at 72 chars
 - please make sure you add spaces after punctuation
 - please make sure you don't send new version of the patch in reply to
   old, add a lore link to previous version under --- instead

About the code - how are you testing this?
What's the packet you're saying we drop?

If transport_header can't be trusted there may be more in this
function to fix..
diff mbox series

Patch

diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 5ebc47da1000..2af98edef87e 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -369,7 +369,7 @@  static int ipv6_frag_rcv(struct sk_buff *skb)
 	 * the source of the fragment, with the Pointer field set to zero.
 	 */
 	nexthdr = hdr->nexthdr;
-	if (ipv6frag_thdr_truncated(skb, skb_transport_offset(skb), &nexthdr)) {
+	if (ipv6frag_thdr_truncated(skb, skb_network_offset(skb) + sizeof(struct ipv6hdr), &nexthdr)) {
 		__IP6_INC_STATS(net, __in6_dev_get_safely(skb->dev),
 				IPSTATS_MIB_INHDRERRORS);
 		icmpv6_param_prob(skb, ICMPV6_HDR_INCOMP, 0);