Skip to content

Commit

Permalink
net: icmp: add skb drop reasons to icmp protocol
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Jiang Biao <[email protected]>
Signed-off-by: Menglong Dong <[email protected]>
Reviewed-by: David Ahern <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
menglongdong authored and davem330 committed Apr 11, 2022
1 parent 41a95a0 commit b384c95
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 47 deletions.
5 changes: 5 additions & 0 deletions include/linux/skbuff.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
2 changes: 1 addition & 1 deletion include/net/ping.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions include/trace/events/skb.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
75 changes: 44 additions & 31 deletions net/ipv4/icmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
};

Expand Down Expand Up @@ -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;
Expand All @@ -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:
Expand Down Expand Up @@ -941,31 +944,31 @@ 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;
}


/*
* 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;
}

/*
Expand All @@ -982,15 +985,15 @@ 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;

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;
Expand All @@ -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.
Expand Down Expand Up @@ -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;
/*
Expand All @@ -1147,45 +1150,50 @@ 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;
}

/*
* Deal with incoming ICMP packets.
*/
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;

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);
}
Expand All @@ -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;
}

/*
Expand All @@ -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
Expand All @@ -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);
Expand Down
14 changes: 8 additions & 6 deletions net/ipv4/ping.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand All @@ -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);

Expand Down
24 changes: 15 additions & 9 deletions net/ipv6/icmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -864,30 +864,35 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)

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;

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);
}
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit b384c95

Please sign in to comment.