From: Qiu Yutan <qiu.yutan@zte.com.cn>

Replace kfree_skb() with kfree_skb_reason() in arp_rcv(). Following
new skb drop reasons are introduced for arp:

/* ARP header hardware address length mismatch */
SKB_DROP_REASON_ARP_HLEN_MISMATCH
/* ARP header protocol addresslength is invalid */
SKB_DROP_REASON_ARP_PLEN_INVALID

Signed-off-by: Qiu Yutan <qiu.yutan@zte.com.cn>
Signed-off-by: Jiang Kun <jiang.kun2@zte.com.cn>
---
 include/net/dropreason-core.h | 12 ++++++++++++
 net/ipv4/arp.c                | 15 ++++++++++++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index bea77934a235..dc846b705c24 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -118,6 +118,8 @@
 	FN(TUNNEL_TXINFO)		\
 	FN(LOCAL_MAC)			\
 	FN(ARP_PVLAN_DISABLE)		\
+	FN(ARP_HLEN_MISMATCH)		\
+	FN(ARP_PLEN_INVALID)		\
 	FN(MAC_IEEE_MAC_CONTROL)	\
 	FN(BRIDGE_INGRESS_STP_STATE)	\
 	FNe(MAX)
@@ -560,6 +562,16 @@ enum skb_drop_reason {
 	 * enabled.
 	 */
 	SKB_DROP_REASON_ARP_PVLAN_DISABLE,
+	/**
+	 * @SKB_DROP_REASON_ARP_HLEN_MISMATCH: ARP header hardware address
+	 * length mismatch.
+	 */
+	SKB_DROP_REASON_ARP_HLEN_MISMATCH,
+	/**
+	 * @SKB_DROP_REASON_ARP_PLEN_INVALID: ARP header protocol address
+	 * length is invalid.
+	 */
+	SKB_DROP_REASON_ARP_PLEN_INVALID,
 	/**
 	 * @SKB_DROP_REASON_MAC_IEEE_MAC_CONTROL: the destination MAC address
 	 * is an IEEE MAC Control address.
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index a648fff71ea7..ca19f2645ccb 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -967,6 +967,7 @@ static int arp_rcv(struct sk_buff *skb, struct net_device *dev,
 		   struct packet_type *pt, struct net_device *orig_dev)
 {
 	const struct arphdr *arp;
+	enum skb_drop_reason drop_reason;

 	/* do not tweak dropwatch on an ARP we will ignore */
 	if (dev->flags & IFF_NOARP ||
@@ -979,12 +980,20 @@ static int arp_rcv(struct sk_buff *skb, struct net_device *dev,
 		goto out_of_mem;

 	/* ARP header, plus 2 device addresses, plus 2 IP addresses.  */
-	if (!pskb_may_pull(skb, arp_hdr_len(dev)))
+	drop_reason = pskb_may_pull_reason(skb, arp_hdr_len(dev));
+	if (drop_reason != SKB_NOT_DROPPED_YET)
 		goto freeskb;

 	arp = arp_hdr(skb);
-	if (arp->ar_hln != dev->addr_len || arp->ar_pln != 4)
+	if (arp->ar_hln != dev->addr_len) {
+		drop_reason = SKB_DROP_REASON_ARP_HLEN_MISMATCH;
 		goto freeskb;
+	}
+
+	if (arp->ar_pln != 4) {
+		drop_reason = SKB_DROP_REASON_ARP_PLEN_INVALID;
+		goto freeskb;
+	}

 	memset(NEIGH_CB(skb), 0, sizeof(struct neighbour_cb));

@@ -996,7 +1005,7 @@ static int arp_rcv(struct sk_buff *skb, struct net_device *dev,
 	consume_skb(skb);
 	return NET_RX_SUCCESS;
 freeskb:
-	kfree_skb(skb);
+	kfree_skb_reason(skb, drop_reason);
 out_of_mem:
 	return NET_RX_DROP;
 }
-- 
2.25.1

On Mon, May 26, 2025 at 1:27 AM <jiang.kun2@zte.com.cn> wrote:
>
> From: Qiu Yutan <qiu.yutan@zte.com.cn>
>
> Replace kfree_skb() with kfree_skb_reason() in arp_rcv(). Following
> new skb drop reasons are introduced for arp:
>
> /* ARP header hardware address length mismatch */
> SKB_DROP_REASON_ARP_HLEN_MISMATCH
> /* ARP header protocol addresslength is invalid */
> SKB_DROP_REASON_ARP_PLEN_INVALID

Are these errors common enough to get dedicated drop reasons ? Most
stacks have implemented ARP more than 20 years ago.

I think that for rare events like this, the standard call graph should
be plenty enough. (perf record -ag -e skb:kfree_skb)

Otherwise we will get 1000 drop reasons, and the profusion of names
makes them useless.

>Are these errors common enough to get dedicated drop reasons ? Most
>stacks have implemented ARP more than 20 years ago.
>
>I think that for rare events like this, the standard call graph should
>be plenty enough. (perf record -ag -e skb:kfree_skb)
>
>Otherwise we will get 1000 drop reasons, and the profusion of names
>makes them useless.

Thank you for your feedback.

Maliciously crafted ARP packets often trigger these two scenarios. 
Using perf cannot directly distinguish between the two cases; 
additionally, enabling perf in embedded environments may lead to 
noticeable performance overhead.

More importantly, in this patch, I believe replacing pskb_may_pull with 
pskb_may_pull_reason makes sense, so using kfree_skb_reason() in 
arp_rcv() is meaningful.

On Mon, May 26, 2025 at 4:47 AM <jiang.kun2@zte.com.cn> wrote:
>
> >Are these errors common enough to get dedicated drop reasons ? Most
> >stacks have implemented ARP more than 20 years ago.
> >
> >I think that for rare events like this, the standard call graph should
> >be plenty enough. (perf record -ag -e skb:kfree_skb)
> >
> >Otherwise we will get 1000 drop reasons, and the profusion of names
> >makes them useless.
>
> Thank you for your feedback.
>
> Maliciously crafted ARP packets often trigger these two scenarios.
> Using perf cannot directly distinguish between the two cases;
> additionally, enabling perf in embedded environments may lead to
> noticeable performance overhead.

ARP packets are local to the domain. They are not a concern, really.

If your local domain has 'malicious hosts', they could actually
trigger more issues by sending
complete packets, to make sure to fill various hash tables.