lkml.org 
[lkml]   [2022]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH net-next 8/9] net: ipv6: add skb drop reasons to ip6_rcv_core()
From

On 4/13/22 01:15, menglong8.dong@gmail.com wrote:
> From: Menglong Dong <imagedong@tencent.com>
>
> Replace kfree_skb() used in ip6_rcv_core() with kfree_skb_reason().
> No new drop reasons are added.
>
> Seems now we use 'SKB_DROP_REASON_IP_INHDR' for too many case during
> ipv6 header parse or check, just like what 'IPSTATS_MIB_INHDRERRORS'
> do. Will it be too general and hard to know what happened?
>
> Signed-off-by: Menglong Dong <imagedong@tencent.com>
> Reviewed-by: Jiang Biao <benbjiang@tencent.com>
> Reviewed-by: Hao Peng <flyingpeng@tencent.com>
> ---
> net/ipv6/ip6_input.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> index b4880c7c84eb..1b925ecb26e9 100644
> --- a/net/ipv6/ip6_input.c
> +++ b/net/ipv6/ip6_input.c
> @@ -145,13 +145,14 @@ static void ip6_list_rcv_finish(struct net *net, struct sock *sk,
> static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
> struct net *net)
> {
> + enum skb_drop_reason reason;
> const struct ipv6hdr *hdr;
> u32 pkt_len;
> struct inet6_dev *idev;
>
> if (skb->pkt_type == PACKET_OTHERHOST) {
> dev_core_stats_rx_otherhost_dropped_inc(skb->dev);
> - kfree_skb(skb);
> + kfree_skb_reason(skb, SKB_DROP_REASON_OTHERHOST);
> return NULL;
> }
>
> @@ -161,9 +162,12 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
>
> __IP6_UPD_PO_STATS(net, idev, IPSTATS_MIB_IN, skb->len);
>
> + SKB_DR_SET(reason, NOT_SPECIFIED);
> if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL ||
> !idev || unlikely(idev->cnf.disable_ipv6)) {
> __IP6_INC_STATS(net, idev, IPSTATS_MIB_INDISCARDS);
> + if (unlikely(idev->cnf.disable_ipv6))

idev can be NULL here (according to surrounding code), and we crash :/



> + SKB_DR_SET(reason, IPV6DISABLED);
> goto drop;
> }
>
> @@ -187,8 +191,10 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
>
> hdr = ipv6_hdr(skb);
>
> - if (hdr->version != 6)
> + if (hdr->version != 6) {
> + SKB_DR_SET(reason, UNHANDLED_PROTO);
> goto err;
> + }
>
> __IP6_ADD_STATS(net, idev,
> IPSTATS_MIB_NOECTPKTS +
> @@ -226,8 +232,10 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
> if (!ipv6_addr_is_multicast(&hdr->daddr) &&
> (skb->pkt_type == PACKET_BROADCAST ||
> skb->pkt_type == PACKET_MULTICAST) &&
> - idev->cnf.drop_unicast_in_l2_multicast)
> + idev->cnf.drop_unicast_in_l2_multicast) {
> + SKB_DR_SET(reason, UNICAST_IN_L2_MULTICAST);
> goto err;
> + }
>
> /* RFC4291 2.7
> * Nodes must not originate a packet to a multicast address whose scope
> @@ -256,12 +264,11 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
> if (pkt_len + sizeof(struct ipv6hdr) > skb->len) {
> __IP6_INC_STATS(net,
> idev, IPSTATS_MIB_INTRUNCATEDPKTS);
> + SKB_DR_SET(reason, PKT_TOO_SMALL);
> goto drop;
> }
> - if (pskb_trim_rcsum(skb, pkt_len + sizeof(struct ipv6hdr))) {
> - __IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
> - goto drop;
> - }
> + if (pskb_trim_rcsum(skb, pkt_len + sizeof(struct ipv6hdr)))
> + goto err;
> hdr = ipv6_hdr(skb);
> }
>
> @@ -282,9 +289,10 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
> return skb;
> err:
> __IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
> + SKB_DR_OR(reason, IP_INHDR);
> drop:
> rcu_read_unlock();
> - kfree_skb(skb);
> + kfree_skb_reason(skb, reason);
> return NULL;
> }
>

\
 
 \ /
  Last update: 2022-04-13 22:42    [W:0.173 / U:1.228 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site