From f54dd4cae9969ecfeabe938f8c308037c1ea59b9 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 14 Oct 2021 05:50:50 -0400 Subject: [PATCH] icmp: fix icmp_ext_echo_iio parsing in icmp_build_probe In icmp_build_probe(), the icmp_ext_echo_iio parsing should be done step by step and skb_header_pointer() return value should always be checked, this patch fixes 3 places in there: - On case ICMP_EXT_ECHO_CTYPE_NAME, it should only copy ident.name from skb by skb_header_pointer(), its len is ident_len. Besides, the return value of skb_header_pointer() should always be checked. - On case ICMP_EXT_ECHO_CTYPE_INDEX, move ident_len check ahead of skb_header_pointer(), and also do the return value check for skb_header_pointer(). - On case ICMP_EXT_ECHO_CTYPE_ADDR, before accessing iio->ident.addr. ctype3_hdr.addrlen, skb_header_pointer() should be called first, then check its return value and ident_len. On subcases ICMP_AFI_IP and ICMP_AFI_IP6, also do check for ident. addr.ctype3_hdr.addrlen and skb_header_pointer()'s return value. On subcase ICMP_AFI_IP, the len for skb_header_pointer() should be "sizeof(iio->extobj_hdr) + sizeof(iio->ident.addr.ctype3_hdr) + sizeof(struct in_addr)" or "ident_len". v1->v2: - To make it more clear, call skb_header_pointer() once only for iio->indent's parsing as Jakub Suggested. v2->v3: - The extobj_hdr.length check against sizeof(_iio) should be done before calling skb_header_pointer(), as Eric noticed. Fixes: 72aea1cc9c46 ("icmp: add response to RFC 8335 PROBE messages") Reported-by: Dan Carpenter Signed-off-by: Xin Long Reviewed-by: Eric Dumazet Link: https://lore.kernel.org/r/31628dd76657ea62f5cf78bb55da6b35240831f1.1634205050.git.lucien.xin@gmail.com Signed-off-by: Jakub Kicinski --- net/ipv4/icmp.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index 8b30cadff708f..b7e277d8a84d2 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -1054,14 +1054,19 @@ bool icmp_build_probe(struct sk_buff *skb, struct icmphdr *icmphdr) iio = skb_header_pointer(skb, sizeof(_ext_hdr), sizeof(iio->extobj_hdr), &_iio); if (!ext_hdr || !iio) goto send_mal_query; - if (ntohs(iio->extobj_hdr.length) <= sizeof(iio->extobj_hdr)) + if (ntohs(iio->extobj_hdr.length) <= sizeof(iio->extobj_hdr) || + ntohs(iio->extobj_hdr.length) > sizeof(_iio)) goto send_mal_query; ident_len = ntohs(iio->extobj_hdr.length) - sizeof(iio->extobj_hdr); + iio = skb_header_pointer(skb, sizeof(_ext_hdr), + sizeof(iio->extobj_hdr) + ident_len, &_iio); + if (!iio) + goto send_mal_query; + status = 0; dev = NULL; switch (iio->extobj_hdr.class_type) { case ICMP_EXT_ECHO_CTYPE_NAME: - iio = skb_header_pointer(skb, sizeof(_ext_hdr), sizeof(_iio), &_iio); if (ident_len >= IFNAMSIZ) goto send_mal_query; memset(buff, 0, sizeof(buff)); @@ -1069,30 +1074,24 @@ bool icmp_build_probe(struct sk_buff *skb, struct icmphdr *icmphdr) dev = dev_get_by_name(net, buff); break; case ICMP_EXT_ECHO_CTYPE_INDEX: - iio = skb_header_pointer(skb, sizeof(_ext_hdr), sizeof(iio->extobj_hdr) + - sizeof(iio->ident.ifindex), &_iio); if (ident_len != sizeof(iio->ident.ifindex)) goto send_mal_query; dev = dev_get_by_index(net, ntohl(iio->ident.ifindex)); break; case ICMP_EXT_ECHO_CTYPE_ADDR: - if (ident_len != sizeof(iio->ident.addr.ctype3_hdr) + + if (ident_len < sizeof(iio->ident.addr.ctype3_hdr) || + ident_len != sizeof(iio->ident.addr.ctype3_hdr) + iio->ident.addr.ctype3_hdr.addrlen) goto send_mal_query; switch (ntohs(iio->ident.addr.ctype3_hdr.afi)) { case ICMP_AFI_IP: - iio = skb_header_pointer(skb, sizeof(_ext_hdr), sizeof(iio->extobj_hdr) + - sizeof(struct in_addr), &_iio); - if (ident_len != sizeof(iio->ident.addr.ctype3_hdr) + - sizeof(struct in_addr)) + if (iio->ident.addr.ctype3_hdr.addrlen != sizeof(struct in_addr)) goto send_mal_query; dev = ip_dev_find(net, iio->ident.addr.ip_addr.ipv4_addr); break; #if IS_ENABLED(CONFIG_IPV6) case ICMP_AFI_IP6: - iio = skb_header_pointer(skb, sizeof(_ext_hdr), sizeof(_iio), &_iio); - if (ident_len != sizeof(iio->ident.addr.ctype3_hdr) + - sizeof(struct in6_addr)) + if (iio->ident.addr.ctype3_hdr.addrlen != sizeof(struct in6_addr)) goto send_mal_query; dev = ipv6_stub->ipv6_dev_find(net, &iio->ident.addr.ip_addr.ipv6_addr, dev); dev_hold(dev); -- 2.39.5