From ac9d8a66e41d553bf57fdffe88f59f1b1443191b Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Wed, 14 Jun 2023 16:01:03 -0700 Subject: [PATCH 1/5] ipv6: rpl: Remove pskb(_may)?_pull() in ipv6_rpl_srh_rcv(). As Eric Dumazet pointed out [0], ipv6_rthdr_rcv() pulls these data - Segment Routing Header : 8 - Hdr Ext Len : skb_transport_header(skb)[1] << 3 needed by ipv6_rpl_srh_rcv(). We can remove pskb_may_pull() and replace pskb_pull() with skb_pull() in ipv6_rpl_srh_rcv(). Link: https://lore.kernel.org/netdev/CANn89iLboLwLrHXeHJucAqBkEL_S0rJFog68t7wwwXO-aNf5Mg@mail.gmail.com/ [0] Signed-off-by: Kuniyuki Iwashima Signed-off-by: Jakub Kicinski --- include/net/rpl.h | 3 --- net/ipv6/exthdrs.c | 17 +---------------- net/ipv6/rpl.c | 7 ------- 3 files changed, 1 insertion(+), 26 deletions(-) diff --git a/include/net/rpl.h b/include/net/rpl.h index 30fe780d1e7c8..74734191c4581 100644 --- a/include/net/rpl.h +++ b/include/net/rpl.h @@ -23,9 +23,6 @@ static inline int rpl_init(void) static inline void rpl_exit(void) {} #endif -size_t ipv6_rpl_srh_size(unsigned char n, unsigned char cmpri, - unsigned char cmpre); - void ipv6_rpl_srh_decompress(struct ipv6_rpl_sr_hdr *outhdr, const struct ipv6_rpl_sr_hdr *inhdr, const struct in6_addr *daddr, unsigned char n); diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c index a543df57801f5..65adc11b59aaf 100644 --- a/net/ipv6/exthdrs.c +++ b/net/ipv6/exthdrs.c @@ -517,11 +517,7 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb) skb_postpull_rcsum(skb, skb_network_header(skb), skb_network_header_len(skb)); - - if (!pskb_pull(skb, offset)) { - kfree_skb(skb); - return -1; - } + skb_pull(skb, offset); skb_postpull_rcsum(skb, skb_transport_header(skb), offset); @@ -543,11 +539,6 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb) return 1; } - if (!pskb_may_pull(skb, sizeof(*hdr))) { - kfree_skb(skb); - return -1; - } - n = (hdr->hdrlen << 3) - hdr->pad - (16 - hdr->cmpre); r = do_div(n, (16 - hdr->cmpri)); /* checks if calculation was without remainder and n fits into @@ -567,12 +558,6 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb) return -1; } - if (!pskb_may_pull(skb, ipv6_rpl_srh_size(n, hdr->cmpri, - hdr->cmpre))) { - kfree_skb(skb); - return -1; - } - hdr->segments_left--; i = n - hdr->segments_left; diff --git a/net/ipv6/rpl.c b/net/ipv6/rpl.c index d1876f1922255..e186998bfbf71 100644 --- a/net/ipv6/rpl.c +++ b/net/ipv6/rpl.c @@ -29,13 +29,6 @@ static void *ipv6_rpl_segdata_pos(const struct ipv6_rpl_sr_hdr *hdr, int i) return (void *)&hdr->rpl_segdata[i * IPV6_PFXTAIL_LEN(hdr->cmpri)]; } -size_t ipv6_rpl_srh_size(unsigned char n, unsigned char cmpri, - unsigned char cmpre) -{ - return sizeof(struct ipv6_rpl_sr_hdr) + (n * IPV6_PFXTAIL_LEN(cmpri)) + - IPV6_PFXTAIL_LEN(cmpre); -} - void ipv6_rpl_srh_decompress(struct ipv6_rpl_sr_hdr *outhdr, const struct ipv6_rpl_sr_hdr *inhdr, const struct in6_addr *daddr, unsigned char n) From 6facbca52da2c209ddffa0c9547769cbcaa268ef Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Wed, 14 Jun 2023 16:01:04 -0700 Subject: [PATCH 2/5] ipv6: rpl: Remove redundant multicast tests in ipv6_rpl_srh_rcv(). ipv6_rpl_srh_rcv() checks if ipv6_hdr(skb)->daddr or ohdr->rpl_segaddr[i] is the multicast address with ipv6_addr_type(). We have the same check for ipv6_hdr(skb)->daddr in ipv6_rthdr_rcv(), so we need not recheck it in ipv6_rpl_srh_rcv(). Also, we should use ipv6_addr_is_multicast() for ohdr->rpl_segaddr[i] instead of ipv6_addr_type(). Signed-off-by: Kuniyuki Iwashima Signed-off-by: Jakub Kicinski --- net/ipv6/exthdrs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c index 65adc11b59aaf..6259e907f0d9f 100644 --- a/net/ipv6/exthdrs.c +++ b/net/ipv6/exthdrs.c @@ -571,8 +571,7 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb) ipv6_rpl_srh_decompress(ohdr, hdr, &ipv6_hdr(skb)->daddr, n); chdr = (struct ipv6_rpl_sr_hdr *)(buf + ((ohdr->hdrlen + 1) << 3)); - if ((ipv6_addr_type(&ipv6_hdr(skb)->daddr) & IPV6_ADDR_MULTICAST) || - (ipv6_addr_type(&ohdr->rpl_segaddr[i]) & IPV6_ADDR_MULTICAST)) { + if (ipv6_addr_is_multicast(&ohdr->rpl_segaddr[i])) { kfree_skb(skb); kfree(buf); return -1; From 0d2e27b858503472f8ed66910498205824b02f8a Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Wed, 14 Jun 2023 16:01:05 -0700 Subject: [PATCH 3/5] ipv6: exthdrs: Replace pskb_pull() with skb_pull() in ipv6_srh_rcv(). ipv6_rthdr_rcv() pulls these data - Segment Routing Header : 8 - Hdr Ext Len : skb_transport_header(skb)[1] << 3 needed by ipv6_srh_rcv(), so pskb_pull() in ipv6_srh_rcv() never fails and can be replaced with skb_pull(). Signed-off-by: Kuniyuki Iwashima Signed-off-by: Jakub Kicinski --- net/ipv6/exthdrs.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c index 6259e907f0d9f..e62e26ac99ef1 100644 --- a/net/ipv6/exthdrs.c +++ b/net/ipv6/exthdrs.c @@ -402,11 +402,7 @@ static int ipv6_srh_rcv(struct sk_buff *skb) skb_postpull_rcsum(skb, skb_network_header(skb), skb_network_header_len(skb)); - - if (!pskb_pull(skb, offset)) { - kfree_skb(skb); - return -1; - } + skb_pull(skb, offset); skb_postpull_rcsum(skb, skb_transport_header(skb), offset); From b83d50f43165e8c21d580f40c57d8f6fe85adb59 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Wed, 14 Jun 2023 16:01:06 -0700 Subject: [PATCH 4/5] ipv6: exthdrs: Reload hdr only when needed in ipv6_srh_rcv(). We need not reload hdr in ipv6_srh_rcv() unless we call pskb_expand_head(). Signed-off-by: Kuniyuki Iwashima Signed-off-by: Jakub Kicinski --- net/ipv6/exthdrs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c index e62e26ac99ef1..dd23531387e3f 100644 --- a/net/ipv6/exthdrs.c +++ b/net/ipv6/exthdrs.c @@ -440,9 +440,9 @@ static int ipv6_srh_rcv(struct sk_buff *skb) kfree_skb(skb); return -1; } - } - hdr = (struct ipv6_sr_hdr *)skb_transport_header(skb); + hdr = (struct ipv6_sr_hdr *)skb_transport_header(skb); + } hdr->segments_left--; addr = hdr->segments + hdr->segments_left; From 6db5dd2bf4817a415daaf6a0226beadef1473d30 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Wed, 14 Jun 2023 16:01:07 -0700 Subject: [PATCH 5/5] ipv6: exthdrs: Remove redundant skb_headlen() check in ip6_parse_tlv(). ipv6_destopt_rcv() and ipv6_parse_hopopts() pulls these data - Hop-by-Hop/Destination Options Header : 8 - Hdr Ext Len : skb_transport_header(skb)[1] << 3 and calls ip6_parse_tlv(), so it need not check if skb_headlen() is less than skb_transport_offset(skb) + (skb_transport_header(skb)[1] << 3). Signed-off-by: Kuniyuki Iwashima Signed-off-by: Jakub Kicinski --- net/ipv6/exthdrs.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c index dd23531387e3f..202fc3aaa83cc 100644 --- a/net/ipv6/exthdrs.c +++ b/net/ipv6/exthdrs.c @@ -126,9 +126,6 @@ static bool ip6_parse_tlv(bool hopbyhop, max_count = -max_count; } - if (skb_transport_offset(skb) + len > skb_headlen(skb)) - goto bad; - off += 2; len -= 2;