net: icmp: add skb drop reasons to icmp protocol
authorMenglong Dong <imagedong@tencent.com>
Thu, 7 Apr 2022 06:20:52 +0000 (14:20 +0800)
committerDavid S. Miller <davem@davemloft.net>
Mon, 11 Apr 2022 09:38:37 +0000 (10:38 +0100)
Replace kfree_skb() used in icmp_rcv() and icmpv6_rcv() with
kfree_skb_reason().

In order to get the reasons of the skb drops after icmp message handle,
we change the return type of 'handler()' in 'struct icmp_control' from
'bool' to 'enum skb_drop_reason'. This may change its original
intention, as 'false' means failure, but 'SKB_NOT_DROPPED_YET' means
success now. Therefore, all 'handler' and the call of them need to be
handled. Following 'handler' functions are involved:

icmp_unreach()
icmp_redirect()
icmp_echo()
icmp_timestamp()
icmp_discard()

And following new drop reasons are added:

SKB_DROP_REASON_ICMP_CSUM
SKB_DROP_REASON_INVALID_PROTO

The reason 'INVALID_PROTO' is introduced for the case that the packet
doesn't follow rfc 1122 and is dropped. This is not a common case, and
I believe we can locate the problem from the data in the packet. For now,
this 'INVALID_PROTO' is used for the icmp broadcasts with wrong types.

Maybe there should be a document file for these reasons. For example,
list all the case that causes the 'UNHANDLED_PROTO' and 'INVALID_PROTO'
drop reason. Therefore, users can locate their problems according to the
document.

Reviewed-by: Hao Peng <flyingpeng@tencent.com>
Reviewed-by: Jiang Biao <benbjiang@tencent.com>
Signed-off-by: Menglong Dong <imagedong@tencent.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/linux/skbuff.h
include/net/ping.h
include/trace/events/skb.h
net/ipv4/icmp.c
net/ipv4/ping.c
net/ipv6/icmp.c

index 173bc35..9b81ba4 100644 (file)
@@ -442,6 +442,11 @@ enum skb_drop_reason {
        SKB_DROP_REASON_TAP_TXFILTER,   /* dropped by tx filter implemented
                                         * at tun/tap, e.g., check_filter()
                                         */
+       SKB_DROP_REASON_ICMP_CSUM,      /* ICMP checksum error */
+       SKB_DROP_REASON_INVALID_PROTO,  /* the packet doesn't follow RFC
+                                        * 2211, such as a broadcasts
+                                        * ICMP_TIMESTAMP
+                                        */
        SKB_DROP_REASON_MAX,
 };
 
index 2fe7887..b68fbfd 100644 (file)
@@ -76,7 +76,7 @@ int  ping_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
 int  ping_common_sendmsg(int family, struct msghdr *msg, size_t len,
                         void *user_icmph, size_t icmph_len);
 int  ping_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
-bool ping_rcv(struct sk_buff *skb);
+enum skb_drop_reason ping_rcv(struct sk_buff *skb);
 
 #ifdef CONFIG_PROC_FS
 void *ping_seq_start(struct seq_file *seq, loff_t *pos, sa_family_t family);
index 85abd7c..4264711 100644 (file)
@@ -61,6 +61,8 @@
        EM(SKB_DROP_REASON_HDR_TRUNC, HDR_TRUNC)                \
        EM(SKB_DROP_REASON_TAP_FILTER, TAP_FILTER)              \
        EM(SKB_DROP_REASON_TAP_TXFILTER, TAP_TXFILTER)          \
+       EM(SKB_DROP_REASON_ICMP_CSUM, ICMP_CSUM)                \
+       EM(SKB_DROP_REASON_INVALID_PROTO, INVALID_PROTO)        \
        EMe(SKB_DROP_REASON_MAX, MAX)
 
 #undef EM
index 72a375c..236debd 100644 (file)
@@ -186,7 +186,7 @@ EXPORT_SYMBOL(icmp_err_convert);
  */
 
 struct icmp_control {
-       bool (*handler)(struct sk_buff *skb);
+       enum skb_drop_reason (*handler)(struct sk_buff *skb);
        short   error;          /* This ICMP is classed as an error message */
 };
 
@@ -839,8 +839,9 @@ static bool icmp_tag_validation(int proto)
  *     ICMP_PARAMETERPROB.
  */
 
-static bool icmp_unreach(struct sk_buff *skb)
+static enum skb_drop_reason icmp_unreach(struct sk_buff *skb)
 {
+       enum skb_drop_reason reason = SKB_NOT_DROPPED_YET;
        const struct iphdr *iph;
        struct icmphdr *icmph;
        struct net *net;
@@ -860,8 +861,10 @@ static bool icmp_unreach(struct sk_buff *skb)
        icmph = icmp_hdr(skb);
        iph   = (const struct iphdr *)skb->data;
 
-       if (iph->ihl < 5) /* Mangled header, drop. */
+       if (iph->ihl < 5)  { /* Mangled header, drop. */
+               reason = SKB_DROP_REASON_IP_INHDR;
                goto out_err;
+       }
 
        switch (icmph->type) {
        case ICMP_DEST_UNREACH:
@@ -941,10 +944,10 @@ static bool icmp_unreach(struct sk_buff *skb)
        icmp_socket_deliver(skb, info);
 
 out:
-       return true;
+       return reason;
 out_err:
        __ICMP_INC_STATS(net, ICMP_MIB_INERRORS);
-       return false;
+       return reason ?: SKB_DROP_REASON_NOT_SPECIFIED;
 }
 
 
@@ -952,20 +955,20 @@ out_err:
  *     Handle ICMP_REDIRECT.
  */
 
-static bool icmp_redirect(struct sk_buff *skb)
+static enum skb_drop_reason icmp_redirect(struct sk_buff *skb)
 {
        if (skb->len < sizeof(struct iphdr)) {
                __ICMP_INC_STATS(dev_net(skb->dev), ICMP_MIB_INERRORS);
-               return false;
+               return SKB_DROP_REASON_PKT_TOO_SMALL;
        }
 
        if (!pskb_may_pull(skb, sizeof(struct iphdr))) {
                /* there aught to be a stat */
-               return false;
+               return SKB_DROP_REASON_NOMEM;
        }
 
        icmp_socket_deliver(skb, ntohl(icmp_hdr(skb)->un.gateway));
-       return true;
+       return SKB_NOT_DROPPED_YET;
 }
 
 /*
@@ -982,7 +985,7 @@ static bool icmp_redirect(struct sk_buff *skb)
  *     See also WRT handling of options once they are done and working.
  */
 
-static bool icmp_echo(struct sk_buff *skb)
+static enum skb_drop_reason icmp_echo(struct sk_buff *skb)
 {
        struct icmp_bxm icmp_param;
        struct net *net;
@@ -990,7 +993,7 @@ static bool icmp_echo(struct sk_buff *skb)
        net = dev_net(skb_dst(skb)->dev);
        /* should there be an ICMP stat for ignored echos? */
        if (net->ipv4.sysctl_icmp_echo_ignore_all)
-               return true;
+               return SKB_NOT_DROPPED_YET;
 
        icmp_param.data.icmph      = *icmp_hdr(skb);
        icmp_param.skb             = skb;
@@ -1001,10 +1004,10 @@ static bool icmp_echo(struct sk_buff *skb)
        if (icmp_param.data.icmph.type == ICMP_ECHO)
                icmp_param.data.icmph.type = ICMP_ECHOREPLY;
        else if (!icmp_build_probe(skb, &icmp_param.data.icmph))
-               return true;
+               return SKB_NOT_DROPPED_YET;
 
        icmp_reply(&icmp_param, skb);
-       return true;
+       return SKB_NOT_DROPPED_YET;
 }
 
 /*     Helper for icmp_echo and icmpv6_echo_reply.
@@ -1122,7 +1125,7 @@ EXPORT_SYMBOL_GPL(icmp_build_probe);
  *               MUST be accurate to a few minutes.
  *               MUST be updated at least at 15Hz.
  */
-static bool icmp_timestamp(struct sk_buff *skb)
+static enum skb_drop_reason icmp_timestamp(struct sk_buff *skb)
 {
        struct icmp_bxm icmp_param;
        /*
@@ -1147,17 +1150,17 @@ static bool icmp_timestamp(struct sk_buff *skb)
        icmp_param.data_len        = 0;
        icmp_param.head_len        = sizeof(struct icmphdr) + 12;
        icmp_reply(&icmp_param, skb);
-       return true;
+       return SKB_NOT_DROPPED_YET;
 
 out_err:
        __ICMP_INC_STATS(dev_net(skb_dst(skb)->dev), ICMP_MIB_INERRORS);
-       return false;
+       return SKB_DROP_REASON_PKT_TOO_SMALL;
 }
 
-static bool icmp_discard(struct sk_buff *skb)
+static enum skb_drop_reason icmp_discard(struct sk_buff *skb)
 {
        /* pretend it was a success */
-       return true;
+       return SKB_NOT_DROPPED_YET;
 }
 
 /*
@@ -1165,18 +1168,20 @@ static bool icmp_discard(struct sk_buff *skb)
  */
 int icmp_rcv(struct sk_buff *skb)
 {
-       struct icmphdr *icmph;
+       enum skb_drop_reason reason = SKB_DROP_REASON_NOT_SPECIFIED;
        struct rtable *rt = skb_rtable(skb);
        struct net *net = dev_net(rt->dst.dev);
-       bool success;
+       struct icmphdr *icmph;
 
        if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
                struct sec_path *sp = skb_sec_path(skb);
                int nh;
 
                if (!(sp && sp->xvec[sp->len - 1]->props.flags &
-                                XFRM_STATE_ICMP))
+                                XFRM_STATE_ICMP)) {
+                       reason = SKB_DROP_REASON_XFRM_POLICY;
                        goto drop;
+               }
 
                if (!pskb_may_pull(skb, sizeof(*icmph) + sizeof(struct iphdr)))
                        goto drop;
@@ -1184,8 +1189,11 @@ int icmp_rcv(struct sk_buff *skb)
                nh = skb_network_offset(skb);
                skb_set_network_header(skb, sizeof(*icmph));
 
-               if (!xfrm4_policy_check_reverse(NULL, XFRM_POLICY_IN, skb))
+               if (!xfrm4_policy_check_reverse(NULL, XFRM_POLICY_IN,
+                                               skb)) {
+                       reason = SKB_DROP_REASON_XFRM_POLICY;
                        goto drop;
+               }
 
                skb_set_network_header(skb, nh);
        }
@@ -1207,13 +1215,13 @@ int icmp_rcv(struct sk_buff *skb)
                /* We can't use icmp_pointers[].handler() because it is an array of
                 * size NR_ICMP_TYPES + 1 (19 elements) and PROBE has code 42.
                 */
-               success = icmp_echo(skb);
-               goto success_check;
+               reason = icmp_echo(skb);
+               goto reason_check;
        }
 
        if (icmph->type == ICMP_EXT_ECHOREPLY) {
-               success = ping_rcv(skb);
-               goto success_check;
+               reason = ping_rcv(skb);
+               goto reason_check;
        }
 
        /*
@@ -1222,8 +1230,10 @@ int icmp_rcv(struct sk_buff *skb)
         *      RFC 1122: 3.2.2  Unknown ICMP messages types MUST be silently
         *                discarded.
         */
-       if (icmph->type > NR_ICMP_TYPES)
+       if (icmph->type > NR_ICMP_TYPES) {
+               reason = SKB_DROP_REASON_UNHANDLED_PROTO;
                goto error;
+       }
 
        /*
         *      Parse the ICMP message
@@ -1239,27 +1249,30 @@ int icmp_rcv(struct sk_buff *skb)
                if ((icmph->type == ICMP_ECHO ||
                     icmph->type == ICMP_TIMESTAMP) &&
                    net->ipv4.sysctl_icmp_echo_ignore_broadcasts) {
+                       reason = SKB_DROP_REASON_INVALID_PROTO;
                        goto error;
                }
                if (icmph->type != ICMP_ECHO &&
                    icmph->type != ICMP_TIMESTAMP &&
                    icmph->type != ICMP_ADDRESS &&
                    icmph->type != ICMP_ADDRESSREPLY) {
+                       reason = SKB_DROP_REASON_INVALID_PROTO;
                        goto error;
                }
        }
 
-       success = icmp_pointers[icmph->type].handler(skb);
-success_check:
-       if (success)  {
+       reason = icmp_pointers[icmph->type].handler(skb);
+reason_check:
+       if (!reason)  {
                consume_skb(skb);
                return NET_RX_SUCCESS;
        }
 
 drop:
-       kfree_skb(skb);
+       kfree_skb_reason(skb, reason);
        return NET_RX_DROP;
 csum_error:
+       reason = SKB_DROP_REASON_ICMP_CSUM;
        __ICMP_INC_STATS(net, ICMP_MIB_CSUMERRORS);
 error:
        __ICMP_INC_STATS(net, ICMP_MIB_INERRORS);
index 9878c76..2897fcf 100644 (file)
@@ -961,12 +961,12 @@ EXPORT_SYMBOL_GPL(ping_queue_rcv_skb);
  *     All we need to do is get the socket.
  */
 
-bool ping_rcv(struct sk_buff *skb)
+enum skb_drop_reason ping_rcv(struct sk_buff *skb)
 {
+       enum skb_drop_reason reason = SKB_DROP_REASON_NO_SOCKET;
        struct sock *sk;
        struct net *net = dev_net(skb->dev);
        struct icmphdr *icmph = icmp_hdr(skb);
-       bool rc = false;
 
        /* We assume the packet has already been checked by icmp_rcv */
 
@@ -981,15 +981,17 @@ bool ping_rcv(struct sk_buff *skb)
                struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
 
                pr_debug("rcv on socket %p\n", sk);
-               if (skb2 && !ping_queue_rcv_skb(sk, skb2))
-                       rc = true;
+               if (skb2)
+                       reason = __ping_queue_rcv_skb(sk, skb2);
+               else
+                       reason = SKB_DROP_REASON_NOMEM;
                sock_put(sk);
        }
 
-       if (!rc)
+       if (reason)
                pr_debug("no socket, dropping\n");
 
-       return rc;
+       return reason;
 }
 EXPORT_SYMBOL_GPL(ping_rcv);
 
index e6b978e..01c8003 100644 (file)
@@ -864,21 +864,23 @@ out:
 
 static int icmpv6_rcv(struct sk_buff *skb)
 {
+       enum skb_drop_reason reason = SKB_DROP_REASON_NOT_SPECIFIED;
        struct net *net = dev_net(skb->dev);
        struct net_device *dev = icmp6_dev(skb);
        struct inet6_dev *idev = __in6_dev_get(dev);
        const struct in6_addr *saddr, *daddr;
        struct icmp6hdr *hdr;
        u8 type;
-       bool success = false;
 
        if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) {
                struct sec_path *sp = skb_sec_path(skb);
                int nh;
 
                if (!(sp && sp->xvec[sp->len - 1]->props.flags &
-                                XFRM_STATE_ICMP))
+                                XFRM_STATE_ICMP)) {
+                       reason = SKB_DROP_REASON_XFRM_POLICY;
                        goto drop_no_count;
+               }
 
                if (!pskb_may_pull(skb, sizeof(*hdr) + sizeof(struct ipv6hdr)))
                        goto drop_no_count;
@@ -886,8 +888,11 @@ static int icmpv6_rcv(struct sk_buff *skb)
                nh = skb_network_offset(skb);
                skb_set_network_header(skb, sizeof(*hdr));
 
-               if (!xfrm6_policy_check_reverse(NULL, XFRM_POLICY_IN, skb))
+               if (!xfrm6_policy_check_reverse(NULL, XFRM_POLICY_IN,
+                                               skb)) {
+                       reason = SKB_DROP_REASON_XFRM_POLICY;
                        goto drop_no_count;
+               }
 
                skb_set_network_header(skb, nh);
        }
@@ -924,11 +929,11 @@ static int icmpv6_rcv(struct sk_buff *skb)
                break;
 
        case ICMPV6_ECHO_REPLY:
-               success = ping_rcv(skb);
+               reason = ping_rcv(skb);
                break;
 
        case ICMPV6_EXT_ECHO_REPLY:
-               success = ping_rcv(skb);
+               reason = ping_rcv(skb);
                break;
 
        case ICMPV6_PKT_TOOBIG:
@@ -994,19 +999,20 @@ static int icmpv6_rcv(struct sk_buff *skb)
        /* until the v6 path can be better sorted assume failure and
         * preserve the status quo behaviour for the rest of the paths to here
         */
-       if (success)
-               consume_skb(skb);
+       if (reason)
+               kfree_skb_reason(skb, reason);
        else
-               kfree_skb(skb);
+               consume_skb(skb);
 
        return 0;
 
 csum_error:
+       reason = SKB_DROP_REASON_ICMP_CSUM;
        __ICMP6_INC_STATS(dev_net(dev), idev, ICMP6_MIB_CSUMERRORS);
 discard_it:
        __ICMP6_INC_STATS(dev_net(dev), idev, ICMP6_MIB_INERRORS);
 drop_no_count:
-       kfree_skb(skb);
+       kfree_skb_reason(skb, reason);
        return 0;
 }