[PATCH net v3] net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

[PATCH net v3] net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

[Star Chang]

Wi-Fi 802.11 to 802.3 frame translation allows attackers to inject
unicast L3 packets or ARP payloads inside GTK-encrypted L2 broadcast
and multicast (BMC) frames (the "Hole-196" vulnerability).

While the `drop_unicast_in_l2_multicast` sysctl was introduced to drop
unicast IPv4/IPv6 packets received over L2 BMC, it had several gaps:

1. ARP packets were not checked in `net/ipv4/arp.c`, allowing attackers
   to perform ARP poisoning via L2 BMC frames (`group-arp-unicast`).
2. Enforcement lacked DHCP client (UDP port 68) exemption, which could
   discard legitimate DHCP server responses sent via L2 broadcast.
3. Dropped packets were silently discarded without warnings or MIB stats.

The issue was identified via manual code inspection and protocol security
analysis of cross-station frame injection in 802.11 shared-key (GTK)
networks. In laboratory testing, crafted 802.11 broadcast frames containing
unicast ARP payloads (e.g. ARPOP_REQUEST / ARPOP_REPLY with tha set to a
victim's unicast MAC) bypassed the existing sysctl, poisoning the victim's
ARP cache. Furthermore, per RFC 2131 inspection, unconditionally dropping
L2 broadcast frames carrying unicast IP addresses discards standard DHCP
server responses (DHCPOFFER/DHCPACK) destined for UDP port 68 during lease
negotiation.

Enhance `drop_unicast_in_l2_multicast` sysctl enforcement:
- Extend sysctl checks to `net/ipv4/arp.c` (`arp_process()`) to drop
  unsolicited ARP Replies and ARP Requests with a non-zero unicast
  Target Hardware Address (tha) received over L2 BMC.
- Exempt Gratuitous ARP (GARP) and RFC 5227 Address Announcements
  (`sip == tip`) from L2 BMC unicast drop logic so they defer to the
  `DROP_GRATUITOUS_ARP` sysctl, preserving VRRP/HSRP router failover.
- Exempt non-fragmented DHCP client traffic (UDP dest port 68) in
  `net/ipv4/ip_input.c` and safely reload `ip_hdr(skb)` after
  `pskb_may_pull()` to prevent dangling pointer access.
- Log rate-limited warnings (`pr_warn_ratelimited`) and increment MIB
  error counters (`IPSTATS_MIB_INHDRERRORS`) for dropped frames.

Fixes: 12b74dfadb5a ("ipv4: add option to drop unicast encapsulated in L2 multicast")
Fixes: abbc30436d39 ("ipv6: add option to drop unicast encapsulated in L2 multicast")
Signed-off-by: Star Chang <starchang@google.com>
---
v2 -> v3:
  - Add Fixes: tags for commits 12b74dfadb5a and abbc30436d39.
  - Reformat lines in net/ipv4/ip_input.c to adhere strictly to the 80-column
    limit.
  - CC maintainers Ido Schimmel and Simon Horman per netdev bot check.
v1 -> v2:
  - Clarified discovery method (manual code inspection and protocol analysis)
    and triggering context in commit message per netdev-bot automated feedback.
 net/ipv4/arp.c       | 29 +++++++++++++++++++++++++++++
 net/ipv4/ip_input.c  | 32 ++++++++++++++++++++++++++++++--
 net/ipv6/ip6_input.c |  2 ++
 3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index d409f606aec0..ad23db2e2f59 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -791,6 +791,35 @@ static int arp_process(struct net *net, struct sock *sk, struct sk_buff *skb)
 	    (!IN_DEV_ROUTE_LOCALNET(in_dev) && ipv4_is_loopback(tip)))
 		goto out_free_skb;
 
+/*
+ * Hole-196 defense for ARP:
+ * If drop_unicast_in_l2_multicast sysctl is enabled on this interface,
+ * drop ARP Replies in L2 BMC frames, and ARP Requests in L2 BMC frames
+ * that specify a non-zero unicast Target Hardware Address (tha).
+ *
+ * Legitimate Gratuitous ARP (GARP) and RFC 5227 Address Announcements
+ * require (sip == tip) and tha matching sha (tha == sha). Only valid
+ * GARP frames are exempted so they defer to DROP_GRATUITOUS_ARP below.
+ */
+	if ((skb->pkt_type == PACKET_BROADCAST ||
+	     skb->pkt_type == PACKET_MULTICAST) &&
+	    IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST)) {
+		bool is_valid_garp = (sip == tip) && tha &&
+				     !memcmp(tha, sha, dev->addr_len);
+
+		if (arp->ar_op == htons(ARPOP_REPLY) && !is_valid_garp) {
+			net_warn_ratelimited("Drop ARP Reply in L2 BMC on %s\n",
+					     dev->name);
+			goto out_free_skb;
+		}
+		if (dev->addr_len == ETH_ALEN && tha &&
+		    is_valid_ether_addr(tha) && !is_valid_garp) {
+			net_warn_ratelimited("Drop unicast THA ARP Req in L2 BMC on %s\n",
+					     dev->name);
+			goto out_free_skb;
+		}
+	}
+
  /*
   *	For some 802.11 wireless deployments (and possibly other networks),
   *	there will be an ARP proxy and gratuitous ARP frames are attacks
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 9860178752b8..75a5c2059632 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -456,10 +456,38 @@ static int ip_rcv_finish_core(struct net *net,
 		 * this is 802.11 protecting against cross-station spoofing (the
 		 * so-called "hole-196" attack) so do it for both.
 		 */
+		/* Hole-196 defense:
+		 * Drop unicast IP packets received over L2 BMC frames.
+		 * Exempt DHCP client responses (UDP port 68).
+		 */
 		if (in_dev &&
 		    IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST)) {
-			drop_reason = SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST;
-			goto drop;
+			bool is_dhcp_resp = false;
+
+			iph = ip_hdr(skb);
+			if (iph->protocol == IPPROTO_UDP &&
+			    !ip_is_fragment(iph) &&
+			    pskb_may_pull(skb, iph->ihl * 4 +
+					       sizeof(struct udphdr))) {
+				const struct udphdr *uh;
+
+				iph = ip_hdr(skb);
+				uh = (const struct udphdr *)
+					(skb_network_header(skb) +
+					 iph->ihl * 4);
+				if (uh->dest == htons(68))
+					is_dhcp_resp = true;
+			}
+
+			if (!is_dhcp_resp) {
+				net_warn_ratelimited("Drop unicast IP %pI4 in L2 BMC on %s
+",
+						     &iph->daddr, dev->name);
+				__IP_INC_STATS(net, IPSTATS_MIB_INHDRERRORS);
+				drop_reason =
+					SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST;
+				goto drop;
+			}
 		}
 	}
 
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 8972863c93ee..caf9811f6352 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -278,6 +278,8 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
 	    (skb->pkt_type == PACKET_BROADCAST ||
 	     skb->pkt_type == PACKET_MULTICAST) &&
 	    READ_ONCE(idev->cnf.drop_unicast_in_l2_multicast)) {
+		net_warn_ratelimited("IPv6: Drop unicast IP %pI6c in L2 BMC on %s\n",
+				     &hdr->daddr, dev->name);
 		SKB_DR_SET(reason, UNICAST_IN_L2_MULTICAST);
 		goto err;
 	}
-- 
2.56.0.257.gf7497c7d27-goog

[PATCH net v4] net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

[Star Chang]

Wi-Fi 802.11 to 802.3 frame translation allows attackers to inject
unicast L3 packets or ARP payloads inside GTK-encrypted L2 broadcast
and multicast (BMC) frames (the "Hole-196" vulnerability).

While the `drop_unicast_in_l2_multicast` sysctl was introduced to drop
unicast IPv4/IPv6 packets received over L2 BMC, it had several gaps:

1. ARP packets were not checked in `net/ipv4/arp.c`, allowing attackers
   to perform ARP poisoning via L2 BMC frames (`group-arp-unicast`).
2. Enforcement lacked DHCP client (UDP port 68) exemption, which could
   discard legitimate DHCP server responses sent via L2 broadcast.
3. Dropped packets were silently discarded without warnings or MIB stats.

The issue was identified via manual code inspection and protocol security
analysis of cross-station frame injection in 802.11 shared-key (GTK)
networks. In laboratory testing, crafted 802.11 broadcast frames containing
unicast ARP payloads (e.g. ARPOP_REQUEST / ARPOP_REPLY with tha set to a
victim's unicast MAC) bypassed the existing sysctl, poisoning the victim's
ARP cache. Furthermore, per RFC 2131 inspection, unconditionally dropping
L2 broadcast frames carrying unicast IP addresses discards standard DHCP
server responses (DHCPOFFER/DHCPACK) destined for UDP port 68 during lease
negotiation.

Enhance `drop_unicast_in_l2_multicast` sysctl enforcement:
- Extend sysctl checks to `net/ipv4/arp.c` (`arp_process()`) to drop
  unsolicited ARP Replies and ARP Requests with a non-zero unicast
  Target Hardware Address (tha) received over L2 BMC.
- Exempt Gratuitous ARP (GARP) and RFC 5227 Address Announcements
  (`sip == tip`) from L2 BMC unicast drop logic so they defer to the
  `DROP_GRATUITOUS_ARP` sysctl, preserving VRRP/HSRP router failover.
- Exempt non-fragmented DHCP client traffic (UDP dest port 68) in
  `net/ipv4/ip_input.c` and safely reload `ip_hdr(skb)` after
  `pskb_may_pull()` to prevent dangling pointer access.
- Log rate-limited warnings (`net_warn_ratelimited`) and increment MIB
  error counters (`IPSTATS_MIB_INHDRERRORS`) for dropped frames.

Fixes: 12b74dfadb5a ("ipv4: add option to drop unicast encapsulated in L2 multicast")
Fixes: abbc30436d39 ("ipv6: add option to drop unicast encapsulated in L2 multicast")
Signed-off-by: Star Chang <starchang@google.com>
---
v3 -> v4:
  - Fix broken newline syntax in net_warn_ratelimited() string literal
    in net/ipv4/ip_input.c that caused build failures across 32-bit,
    allmodconfig, and clang CI builds.
  - CC Julian Anastasov (ja@ssi.bg) per netdev-bot blamed author check.
v2 -> v3:
  - Add Fixes: tags for commits 12b74dfadb5a and abbc30436d39.
  - Reformat lines in net/ipv4/ip_input.c to adhere strictly to the 80-column limit.
  - CC maintainers Ido Schimmel and Simon Horman per netdev bot check.
v1 -> v2:
  - Clarified discovery method and triggering context per netdev-bot feedback.

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index d409f606aec0..ad23db2e2f59 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -791,6 +791,35 @@ static int arp_process(struct net *net, struct sock *sk, struct sk_buff *skb)
 	    (!IN_DEV_ROUTE_LOCALNET(in_dev) && ipv4_is_loopback(tip)))
 		goto out_free_skb;
 
+/*
+ * Hole-196 defense for ARP:
+ * If drop_unicast_in_l2_multicast sysctl is enabled on this interface,
+ * drop ARP Replies in L2 BMC frames, and ARP Requests in L2 BMC frames
+ * that specify a non-zero unicast Target Hardware Address (tha).
+ *
+ * Legitimate Gratuitous ARP (GARP) and RFC 5227 Address Announcements
+ * require (sip == tip) and tha matching sha (tha == sha). Only valid
+ * GARP frames are exempted so they defer to DROP_GRATUITOUS_ARP below.
+ */
+	if ((skb->pkt_type == PACKET_BROADCAST ||
+	     skb->pkt_type == PACKET_MULTICAST) &&
+	    IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST)) {
+		bool is_valid_garp = (sip == tip) && tha &&
+				     !memcmp(tha, sha, dev->addr_len);
+
+		if (arp->ar_op == htons(ARPOP_REPLY) && !is_valid_garp) {
+			net_warn_ratelimited("Drop ARP Reply in L2 BMC on %s\n",
+					     dev->name);
+			goto out_free_skb;
+		}
+		if (dev->addr_len == ETH_ALEN && tha &&
+		    is_valid_ether_addr(tha) && !is_valid_garp) {
+			net_warn_ratelimited("Drop unicast THA ARP Req in L2 BMC on %s\n",
+					     dev->name);
+			goto out_free_skb;
+		}
+	}
+
  /*
   *	For some 802.11 wireless deployments (and possibly other networks),
   *	there will be an ARP proxy and gratuitous ARP frames are attacks
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 9860178752b8..7d6d3e72e557 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -456,10 +456,37 @@ static int ip_rcv_finish_core(struct net *net,
 		 * this is 802.11 protecting against cross-station spoofing (the
 		 * so-called "hole-196" attack) so do it for both.
 		 */
+		/* Hole-196 defense:
+		 * Drop unicast IP packets received over L2 BMC frames.
+		 * Exempt DHCP client responses (UDP port 68).
+		 */
 		if (in_dev &&
 		    IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST)) {
-			drop_reason = SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST;
-			goto drop;
+			bool is_dhcp_resp = false;
+
+			iph = ip_hdr(skb);
+			if (iph->protocol == IPPROTO_UDP &&
+			    !ip_is_fragment(iph) &&
+			    pskb_may_pull(skb, iph->ihl * 4 +
+					       sizeof(struct udphdr))) {
+				const struct udphdr *uh;
+
+				iph = ip_hdr(skb);
+				uh = (const struct udphdr *)
+					(skb_network_header(skb) +
+					 iph->ihl * 4);
+				if (uh->dest == htons(68))
+					is_dhcp_resp = true;
+			}
+
+			if (!is_dhcp_resp) {
+				net_warn_ratelimited("Drop unicast IP %pI4 in L2 BMC on %s\n",
+						     &iph->daddr, dev->name);
+				__IP_INC_STATS(net, IPSTATS_MIB_INHDRERRORS);
+				drop_reason =
+					SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST;
+				goto drop;
+			}
 		}
 	}
 
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 8972863c93ee..caf9811f6352 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -278,6 +278,8 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
 	    (skb->pkt_type == PACKET_BROADCAST ||
 	     skb->pkt_type == PACKET_MULTICAST) &&
 	    READ_ONCE(idev->cnf.drop_unicast_in_l2_multicast)) {
+		net_warn_ratelimited("IPv6: Drop unicast IP %pI6c in L2 BMC on %s\n",
+				     &hdr->daddr, dev->name);
 		SKB_DR_SET(reason, UNICAST_IN_L2_MULTICAST);
 		goto err;
 	}

-- 
2.43.0

Re: [PATCH net v4] net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

[Julian Anastasov]

	Hello,

On Tue, 27 Oct 2026, Star Chang wrote:

> Wi-Fi 802.11 to 802.3 frame translation allows attackers to inject
> unicast L3 packets or ARP payloads inside GTK-encrypted L2 broadcast
> and multicast (BMC) frames (the "Hole-196" vulnerability).
> 
> While the `drop_unicast_in_l2_multicast` sysctl was introduced to drop
> unicast IPv4/IPv6 packets received over L2 BMC, it had several gaps:
> 
> 1. ARP packets were not checked in `net/ipv4/arp.c`, allowing attackers
>    to perform ARP poisoning via L2 BMC frames (`group-arp-unicast`).
> 2. Enforcement lacked DHCP client (UDP port 68) exemption, which could
>    discard legitimate DHCP server responses sent via L2 broadcast.
> 3. Dropped packets were silently discarded without warnings or MIB stats.
> 
> The issue was identified via manual code inspection and protocol security
> analysis of cross-station frame injection in 802.11 shared-key (GTK)
> networks. In laboratory testing, crafted 802.11 broadcast frames containing
> unicast ARP payloads (e.g. ARPOP_REQUEST / ARPOP_REPLY with tha set to a
> victim's unicast MAC) bypassed the existing sysctl, poisoning the victim's
> ARP cache. Furthermore, per RFC 2131 inspection, unconditionally dropping
> L2 broadcast frames carrying unicast IP addresses discards standard DHCP
> server responses (DHCPOFFER/DHCPACK) destined for UDP port 68 during lease
> negotiation.
> 
> Enhance `drop_unicast_in_l2_multicast` sysctl enforcement:
> - Extend sysctl checks to `net/ipv4/arp.c` (`arp_process()`) to drop
>   unsolicited ARP Replies and ARP Requests with a non-zero unicast
>   Target Hardware Address (tha) received over L2 BMC.
> - Exempt Gratuitous ARP (GARP) and RFC 5227 Address Announcements
>   (`sip == tip`) from L2 BMC unicast drop logic so they defer to the
>   `DROP_GRATUITOUS_ARP` sysctl, preserving VRRP/HSRP router failover.
> - Exempt non-fragmented DHCP client traffic (UDP dest port 68) in
>   `net/ipv4/ip_input.c` and safely reload `ip_hdr(skb)` after
>   `pskb_may_pull()` to prevent dangling pointer access.
> - Log rate-limited warnings (`net_warn_ratelimited`) and increment MIB
>   error counters (`IPSTATS_MIB_INHDRERRORS`) for dropped frames.
> 
> Fixes: 12b74dfadb5a ("ipv4: add option to drop unicast encapsulated in L2 multicast")
> Fixes: abbc30436d39 ("ipv6: add option to drop unicast encapsulated in L2 multicast")
> Signed-off-by: Star Chang <starchang@google.com>
> ---
> v3 -> v4:
>   - Fix broken newline syntax in net_warn_ratelimited() string literal
>     in net/ipv4/ip_input.c that caused build failures across 32-bit,
>     allmodconfig, and clang CI builds.
>   - CC Julian Anastasov (ja@ssi.bg) per netdev-bot blamed author check.
> v2 -> v3:
>   - Add Fixes: tags for commits 12b74dfadb5a and abbc30436d39.
>   - Reformat lines in net/ipv4/ip_input.c to adhere strictly to the 80-column limit.
>   - CC maintainers Ido Schimmel and Simon Horman per netdev bot check.
> v1 -> v2:
>   - Clarified discovery method and triggering context per netdev-bot feedback.
> 
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index d409f606aec0..ad23db2e2f59 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -791,6 +791,35 @@ static int arp_process(struct net *net, struct sock *sk, struct sk_buff *skb)
>  	    (!IN_DEV_ROUTE_LOCALNET(in_dev) && ipv4_is_loopback(tip)))
>  		goto out_free_skb;
>  
> +/*
> + * Hole-196 defense for ARP:
> + * If drop_unicast_in_l2_multicast sysctl is enabled on this interface,
> + * drop ARP Replies in L2 BMC frames, and ARP Requests in L2 BMC frames
> + * that specify a non-zero unicast Target Hardware Address (tha).
> + *
> + * Legitimate Gratuitous ARP (GARP) and RFC 5227 Address Announcements
> + * require (sip == tip) and tha matching sha (tha == sha). Only valid
> + * GARP frames are exempted so they defer to DROP_GRATUITOUS_ARP below.
> + */
> +	if ((skb->pkt_type == PACKET_BROADCAST ||
> +	     skb->pkt_type == PACKET_MULTICAST) &&
> +	    IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST)) {
> +		bool is_valid_garp = (sip == tip) && tha &&
> +				     !memcmp(tha, sha, dev->addr_len);

	I'm not sure how much from the Sashiko review is addressed:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261002082336.608201-1-starchang%40google.com

	But matching sha and tha for requests is wrong.

From RFC 826:

===
The target hardware address is included for completeness and
network monitoring.  It has no meaning in the request form, since
it is this number that the machine is requesting.  Its meaning in
the reply form is the address of the machine making the request.
===

	tha is used only for monitoring. Linux copies sha from
requests into tha field in the reply. It does not use the tha
to update the cached entry, instead sha is used for this.

	From RFC 2002, section 4.6, about Gratuitous ARP:

===
When using an ARP Reply packet, the
Target Hardware Address is also set to the link-layer address to
which this cache entry should be updated (this field is not used
in an ARP Request packet).
===

	It means to store the sender address there too. But as we
see nobody updates cache from tha, only from sha. But there are
GARP implementations that store broadcast address in tha when
sending broadcast GARP reply, may be because the packet is intended
to ALL, not just to simulate a reply packet addressed to the sender.

	So, I'm not sure why arp_is_garp() requires tha and
sha to match for GARP reply. It is too restrictive for no good
reason. We can safely ingore tha in arp_process. Let the packet
capture tools to check it, for monitoring purposes. There we can
see in tha to which hosts the packet was sent.

	As for the ARP broadcasts, we already have "arp_accept"
sysctl that prevents new entries to be added to cache. Can it
help for your ARP tests?

> +
> +		if (arp->ar_op == htons(ARPOP_REPLY) && !is_valid_garp) {
> +			net_warn_ratelimited("Drop ARP Reply in L2 BMC on %s\n",
> +					     dev->name);
> +			goto out_free_skb;
> +		}
> +		if (dev->addr_len == ETH_ALEN && tha &&
> +		    is_valid_ether_addr(tha) && !is_valid_garp) {
> +			net_warn_ratelimited("Drop unicast THA ARP Req in L2 BMC on %s\n",
> +					     dev->name);
> +			goto out_free_skb;
> +		}
> +	}

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH net v4] net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

[Star Chang]

Hi Julian,

Thank you very much for the insightful review and for pointing out the RFC 826 and RFC 2002 references.

You are completely right on both points:
1. Per RFC 826, the Target Hardware Address (tha) in an ARP Request has no protocol meaning, and Linux updates neighbor entries using (sip, sha), never tha. Inspecting tha on ARP Requests was a fundamental mistake and would break legitimate devices that leave arbitrary/unicast addresses in tha.
2. In RFC 2002 (section 4.6), many real-world GARP reply implementations legitimately set tha to the broadcast address (ff:ff:ff:ff:ff:ff) since the announcement is broadcast to all hosts. Requiring tha == sha in our check was overly restrictive and would cause false-positive drops for legitimate GARP replies.

Regarding your question about arp_accept:
In our testing, arp_accept = 0 successfully prevents creating *new* neighbor entries from unsolicited broadcasts. However, in typical client scenarios, the victim station almost always already has an existing neighbor entry for its default gateway (i.e. n is not NULL in arp_process).

When n already exists, arp_accept is bypassed, and once the 1-second LOCKTIME window has passed, arp_process() calls:
    neigh_update(n, sha, state, NEIGH_UPDATE_F_OVERRIDE, 0);
which executes memcpy(&neigh->ha, lladdr, dev->addr_len) in __neigh_update(), overwriting the gateway's cached MAC address with the attacker's MAC.

In a Wi-Fi (Hole-196) scenario where clients share the GTK, an attacker cannot forge unicast frames directly to the victim, so they broadcast an unsolicited unicast ARP Reply (sip = gateway_ip, sha = attacker_mac, tip = victim_ip) over L2 broadcast (PACKET_BROADCAST).

Under standard networking behavior, a non-GARP ARP Reply is strictly meant to be delivered as an L2 unicast frame (PACKET_HOST) to the requester. The only legitimate reason an ARP Reply is ever sent over L2 broadcast is for Gratuitous ARP (sip == tip).

Based on your feedback, we plan to completely drop all checks on tha, leave ARP Requests entirely untouched, and strictly narrow the ARP defense to drop ARP Replies received over L2 BMC *only* if they are not Gratuitous ARP (sip != tip):

--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -791,32 +791,18 @@ static int arp_process(struct net *net, struct sock *sk, struct sk_buff *skb)
 	    (!IN_DEV_ROUTE_LOCALNET(in_dev) && ipv4_is_loopback(tip)))
 		goto out_free_skb;
 
 	if ((skb->pkt_type == PACKET_BROADCAST ||
 	     skb->pkt_type == PACKET_MULTICAST) &&
 	    IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST)) {
-		bool is_valid_garp = (sip == tip) && tha &&
-				     !memcmp(tha, sha, dev->addr_len);
-
-		if (arp->ar_op == htons(ARPOP_REPLY) && !is_valid_garp) {
-			net_warn_ratelimited("Drop ARP Reply in L2 BMC on %s\n",
+		/*
+		 * Standard ARP Replies are strictly unicast (PACKET_HOST).
+		 * An ARP Reply in an L2 broadcast frame with (sip != tip)
+		 * is an injected unicast reply. Legitimate GARP (sip == tip)
+		 * is preserved and defers to DROP_GRATUITOUS_ARP.
+		 * ARP Requests are left untouched per RFC 826.
+		 */
+		if (arp->ar_op == htons(ARPOP_REPLY) && sip != tip) {
+			net_warn_ratelimited("Drop non-GARP ARP Reply in L2 BMC on %s\n",
 					     dev->name);
 			goto out_free_skb;
 		}
-		if (dev->addr_len == ETH_ALEN && tha &&
-		    is_valid_ether_addr(tha) && !is_valid_garp) {
-			net_warn_ratelimited("Drop unicast THA ARP Req in L2 BMC on %s\n",
-					     dev->name);
-			goto out_free_skb;
-		}
 	}

With this approach:
1. ARP Requests are completely untouched (fully adhering to RFC 826).
2. Legitimate GARP announcements (sip == tip) for VRRP/HSRP router failovers are preserved regardless of what value they put in tha (respecting RFC 2002).
3. Injected unicast ARP Replies in L2 BMC (sip != tip) that poison existing gateway entries are dropped.

Does this sound like the right direction to you for v5?

Thanks again for your time, guidance, and expertise!

Best regards,
Star Chang

Re: [PATCH net v4] net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

[Julian Anastasov]

	Hello,

On Wed, 28 Oct 2026, Star Chang wrote:

> Hi Julian,
> 
> Thank you very much for the insightful review and for pointing out the RFC 826 and RFC 2002 references.
> 
> You are completely right on both points:
> 1. Per RFC 826, the Target Hardware Address (tha) in an ARP Request has no protocol meaning, and Linux updates neighbor entries using (sip, sha), never tha. Inspecting tha on ARP Requests was a fundamental mistake and would break legitimate devices that leave arbitrary/unicast addresses in tha.
> 2. In RFC 2002 (section 4.6), many real-world GARP reply implementations legitimately set tha to the broadcast address (ff:ff:ff:ff:ff:ff) since the announcement is broadcast to all hosts. Requiring tha == sha in our check was overly restrictive and would cause false-positive drops for legitimate GARP replies.
> 
> Regarding your question about arp_accept:
> In our testing, arp_accept = 0 successfully prevents creating *new* neighbor entries from unsolicited broadcasts. However, in typical client scenarios, the victim station almost always already has an existing neighbor entry for its default gateway (i.e. n is not NULL in arp_process).
> 
> When n already exists, arp_accept is bypassed, and once the 1-second LOCKTIME window has passed, arp_process() calls:
>     neigh_update(n, sha, state, NEIGH_UPDATE_F_OVERRIDE, 0);
> which executes memcpy(&neigh->ha, lladdr, dev->addr_len) in __neigh_update(), overwriting the gateway's cached MAC address with the attacker's MAC.
> 
> In a Wi-Fi (Hole-196) scenario where clients share the GTK, an attacker cannot forge unicast frames directly to the victim, so they broadcast an unsolicited unicast ARP Reply (sip = gateway_ip, sha = attacker_mac, tip = victim_ip) over L2 broadcast (PACKET_BROADCAST).
> 
> Under standard networking behavior, a non-GARP ARP Reply is strictly meant to be delivered as an L2 unicast frame (PACKET_HOST) to the requester. The only legitimate reason an ARP Reply is ever sent over L2 broadcast is for Gratuitous ARP (sip == tip).
> 
> Based on your feedback, we plan to completely drop all checks on tha, leave ARP Requests entirely untouched, and strictly narrow the ARP defense to drop ARP Replies received over L2 BMC *only* if they are not Gratuitous ARP (sip != tip):

	Sashiko gives also example with RFC 3927 Section 2.5 and 4
where ARP packets (especially replies) with sip in 169.254.0.0/16
MUST be sent as broadcasts, so that boxes can detect that
their address is configured also on another host. So, may be
we should not drop sip in 169.254.0.0/16. But it is another open door.

	As for the received broadcast requests, it is still possible
to send replies where needed but to skip the step where we use sha
to update the cache for sip. We can set some flag 'cacheupd' early
and later to check it. The result will be:

- ARP Probes: replied as usually (there is no cache update for sip 0)
- n = neigh_event_ns(): avoided but replies are sent if !dont_send
- do not lookup for n, jump before the lookup
- ARP Announce (GARP): ignored, sorry, the attacker announces
new sha for GWIP

> 
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -791,32 +791,18 @@ static int arp_process(struct net *net, struct sock *sk, struct sk_buff *skb)
>  	    (!IN_DEV_ROUTE_LOCALNET(in_dev) && ipv4_is_loopback(tip)))
>  		goto out_free_skb;
>  
>  	if ((skb->pkt_type == PACKET_BROADCAST ||
>  	     skb->pkt_type == PACKET_MULTICAST) &&
>  	    IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST)) {
> -		bool is_valid_garp = (sip == tip) && tha &&
> -				     !memcmp(tha, sha, dev->addr_len);
> -
> -		if (arp->ar_op == htons(ARPOP_REPLY) && !is_valid_garp) {
> -			net_warn_ratelimited("Drop ARP Reply in L2 BMC on %s\n",
> +		/*
> +		 * Standard ARP Replies are strictly unicast (PACKET_HOST).
> +		 * An ARP Reply in an L2 broadcast frame with (sip != tip)
> +		 * is an injected unicast reply. Legitimate GARP (sip == tip)
> +		 * is preserved and defers to DROP_GRATUITOUS_ARP.
> +		 * ARP Requests are left untouched per RFC 826.
> +		 */
> +		if (arp->ar_op == htons(ARPOP_REPLY) && sip != tip) {
> +			net_warn_ratelimited("Drop non-GARP ARP Reply in L2 BMC on %s\n",
>  					     dev->name);
>  			goto out_free_skb;
>  		}
> -		if (dev->addr_len == ETH_ALEN && tha &&
> -		    is_valid_ether_addr(tha) && !is_valid_garp) {
> -			net_warn_ratelimited("Drop unicast THA ARP Req in L2 BMC on %s\n",
> -					     dev->name);
> -			goto out_free_skb;
> -		}
>  	}
> 
> With this approach:
> 1. ARP Requests are completely untouched (fully adhering to RFC 826).

	See above

> 2. Legitimate GARP announcements (sip == tip) for VRRP/HSRP router failovers are preserved regardless of what value they put in tha (respecting RFC 2002).

	Drop them. Or leave it to DROP_GRATUITOUS_ARP to decide,
it is per iface too. There should be no chance to update cache
from broadcasts.

> 3. Injected unicast ARP Replies in L2 BMC (sip != tip) that poison existing gateway entries are dropped.
> 
> Does this sound like the right direction to you for v5?

	Yep. But not sure for the IP part...

> Thanks again for your time, guidance, and expertise!
> 
> Best regards,
> Star Chang

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH net v4] net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

[Star Chang]

Hi Julian,

Thank you for the prompt feedback!

On the ARP side:
Glad to hear that dropping non-GARP ARP Replies (ar_op == ARPOP_REPLY && sip != tip) in L2 BMC sounds like the right direction.

Regarding the points you raised:
1. For GARP: Leaving it to DROP_GRATUITOUS_ARP to decide makes total sense. Legitimate GARP (sip == tip) will bypass our drop check and fall through to the existing IN_DEV_DROP_GRATUITOUS_ARP logic in arp_process().
2. For RFC 3927 (169.254.0.0/16): I completely agree with your concern that exempting it opens another door. Given that Hole-196 attacks typically target the subnet gateway rather than link-local addresses, keeping it simple without an exemption seems safer.

Regarding your comment on the IP part ("But not sure for the IP part..."):
Could you please elaborate on your concerns with the IP changes?

The motivation for exempting UDP port 68 was that in our lab testing, we noticed certain DHCP servers responding with unicast IP addresses over L2 broadcast, which caused legitimate lease packets to be dropped by drop_unicast_in_l2_multicast.

We would be very glad to adjust, simplify, or rethink the IP logic based on your advice.

Thanks again for your time and guidance!

Best regards,
Star Chang

Re: [PATCH net v4] net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

[Julian Anastasov]

	Hello,

On Thu, 29 Oct 2026, Star Chang wrote:

> Hi Julian,
> 
> Thank you for the prompt feedback!
> 
> On the ARP side:
> Glad to hear that dropping non-GARP ARP Replies (ar_op == ARPOP_REPLY && sip != tip) in L2 BMC sounds like the right direction.
> 
> Regarding the points you raised:
> 1. For GARP: Leaving it to DROP_GRATUITOUS_ARP to decide makes total sense. Legitimate GARP (sip == tip) will bypass our drop check and fall through to the existing IN_DEV_DROP_GRATUITOUS_ARP logic in arp_process().

	May be DROP_UNICAST_IN_L2_MULTICAST should drop also
the GARP packets. Why should the GARP packets to be more
privileged than the other broadcast requests?

	I see your v5 and it looks like you do not take the
path to stop the broadcast requests from updating the cache.

> 2. For RFC 3927 (169.254.0.0/16): I completely agree with your concern that exempting it opens another door. Given that Hole-196 attacks typically target the subnet gateway rather than link-local addresses, keeping it simple without an exemption seems safer.

	As this is a broadcast reply, we should be concerned
if the same IP is locally configured. But what if we send
broadcast request and receive broadcast reply for such address?
I don't know if there is some support for such broadcasts.

	As result, IMHO the handling should be:

- broadcast request: do not update cache, send replies as usually.
GARP requests (ARP Announcement) should not update the cache.

- broadcast replies:
	- reply for 169.254.0.0/16: one day these should be
validated and then allowed, now dropped
	- otherwise, drop, do not update cache

- the ARP traffic on the WLAN will increase because we will not
update the cache from remote requests, only from remote unicast
replies which are in response to our broadcast requests

> Regarding your comment on the IP part ("But not sure for the IP part..."):
> Could you please elaborate on your concerns with the IP changes?

	From my notes about DHCPv4, I see broadcasts only for:

- DISCOVER: client_mac->brd, 0.0.0.0:68 -> 255.255.255.255:67
- REQUEST: UNUSED IP, client_mac->brd, 0.0.0.0:68 -> 255.255.255.255:67

	But things now may differ.
> 
> The motivation for exempting UDP port 68 was that in our lab testing, we noticed certain DHCP servers responding with unicast IP addresses over L2 broadcast, which caused legitimate lease packets to be dropped by drop_unicast_in_l2_multicast.
> 
> We would be very glad to adjust, simplify, or rethink the IP logic based on your advice.
> 
> Thanks again for your time and guidance!
> 
> Best regards,
> Star Chang

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH net v4] net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

[Star Chang]

Hi Julian, Sashiko, and netdev reviewers,

Thank you for the detailed review and feedback.

The points raised regarding the IP part are completely legitimate:
1. The pskb_may_pull() failure path did indeed introduce a potential UAF read of &iph->daddr when skb->head was reallocated.
2. Furthermore, exempting UDP port 68 weakens the Hole-196 protection. In practice, DHCP clients operate via AF_PACKET raw sockets (which tap frames before IP input) or rely on broadcast destination 255.255.255.255 (which routes as RTN_BROADCAST and is never dropped by drop_unicast_in_l2_multicast).
3. The net_warn_ratelimited() calls also introduce potential log suppression risks under a packet flood.

Therefore, we have completely dropped the IP and IPv6 changes (ip_input.c and ip6_input.c), preserving the original silent discard behavior per RFC 1122 and removing the extraneous IPv6 Fixes tag.

On the ARP side:
We have simplified the logic per the discussion with Julian:
- Strictly drop ARP Replies in L2 BMC unless they are Gratuitous ARP (sip == tip), recording SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST via kfree_skb_reason().
- Leave ARP Requests completely untouched per RFC 826 (eliminating any checks on tha).
- Legitimate GARP (sip == tip) defers to the existing drop_gratuitous_arp sysctl.
- Updated Documentation/networking/ip-sysctl.rst accordingly.

We will submit these changes cleanly in v5.

pw-bot: cr

Re: [PATCH net v4] net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

[Star Chang]

Hi Julian,

Thank you! Your point is spot-on: in a shared-key Wi-Fi network, an attacker
can simply broadcast a forged GARP reply (sip == tip == gateway_ip) to poison
the gateway MAC for all peers. Giving GARP an exemption indeed leaves a door open.

We verified the Android / mobile use case and confirmed that completely
disallowing neighbor learning from L2 BMC is safe:
- Gateway validation: Android relies on NUD (Neighbor Unreachability Detection)
  unicast probes and unicast replies (PACKET_HOST), never on broadcast replies.
- Wi-Fi Roaming: Roaming clients *send* GARP to notify the AP/switch; they do
  not need to receive broadcast GARP.
- Router failover: NUD naturally detects stale neighbor state and re-probes
  via unicast, recovering within seconds.

For v6, following your guidance:
1. L2 BMC ARP Replies:
   Drop all of them unconditionally (including GARP) with
   SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST.
2. L2 BMC ARP Requests:
   Process and send replies as usual, but suppress updating/creating neighbor
   cache entries.

Does this match your vision for v6? If so, I will prepare the patch accordingly.

Best regards,
Star Chang

Re: [PATCH net v4] net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 10 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 2 · Low: 6

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The ARP side of the Hole-196 fix can be bypassed by changing one
  field.
- [High] Use-after-free read in ip_rcv_finish_core()
  (net/ipv4/ip_input.c:467-484).
- [Medium] The new DHCP exemption in ip_rcv_finish_core()
  (net/ipv4/ip_input.c:463-490) weakens the existing Hole-196 drop, and…
- [Medium] Turning on drop_unicast_in_l2_multicast now drops every ARP
  Reply that arrives in an L2 broadcast/multicast frame unless it…
- [Low] The patch changes the behaviour of the per-interface IPv4 sysctl
  drop_unicast_in_l2_multicast without updating…
- [Low] The commit message misstates the existing behaviour, and the IPv6
  hunk tagged as a fix only adds a printk.
- [Low] The new comment and the `is_valid_garp` predicate in
  arp_process() define GARP differently from the GARP helper already in…
- [Low] The three places that enforce drop_unicast_in_l2_multicast report
  drops differently.
- [Low] The IPv4 and IPv6 sysctls named drop_unicast_in_l2_multicast now
  behave differently, while their documentation (ip-sysctl.rst:2303 and…
- [Low] The patch adds net_warn_ratelimited() on four drop paths, and any
  L2 peer can trigger all of them at line rate with crafted frames:…

--

net: Fix Hole-196 vulnerability by dropping unicast ARP/IP in L2 BMC

This patch extends the drop_unicast_in_l2_multicast sysctl so that it also
drops ARP replies and unicast-THA ARP requests that arrive in L2
broadcast/multicast frames, with an exemption for gratuitous ARP. It also
exempts non-fragmented UDP traffic to port 68 from the IPv4 drop. It adds
rate-limited warnings and an IPv4 MIB counter on the drop paths.

> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 1ca4ced280aef..9f6715a5b5475 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -794,6 +794,35 @@ static int arp_process(struct net *net, struct sock *sk, struct sk_buff *skb)
>  	    (!IN_DEV_ROUTE_LOCALNET(in_dev) && ipv4_is_loopback(tip)))
>  		goto out_free_skb;
>  
> +/*
> + * Hole-196 defense for ARP:
> + * If drop_unicast_in_l2_multicast sysctl is enabled on this interface,
> + * drop ARP Replies in L2 BMC frames, and ARP Requests in L2 BMC frames
> + * that specify a non-zero unicast Target Hardware Address (tha).
> + *
> + * Legitimate Gratuitous ARP (GARP) and RFC 5227 Address Announcements
> + * require (sip == tip) and tha matching sha (tha == sha). Only valid
> + * GARP frames are exempted so they defer to DROP_GRATUITOUS_ARP below.
> + */
> +	if ((skb->pkt_type == PACKET_BROADCAST ||
> +	     skb->pkt_type == PACKET_MULTICAST) &&
> +	    IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST)) {

[Severity: Low]
Should Documentation/networking/ip-sysctl.rst be updated as well? The IPv4
entry still says:

    drop_unicast_in_l2_multicast - BOOLEAN
	Drop any unicast IP packets that are received in link-layer
	multicast (or broadcast) frames.

After this patch the same knob also filters ARP in arp_process(). It also
no longer drops every unicast IP packet in ip_rcv_finish_core(), because
UDP datagrams to port 68 are now let through.

> +		bool is_valid_garp = (sip == tip) && tha &&
> +				     !memcmp(tha, sha, dev->addr_len);

[Severity: Low]
Does this GARP definition match arp_is_garp() in the same file? The
existing helper only requires tha == sha for replies:

net/ipv4/arp.c:arp_is_garp() {
	bool is_garp = tip == sip;

	/* Gratuitous ARP _replies_ also require target hwaddr to be
	 * the same as source.
	 */
	if (is_garp && ar_op == htons(ARPOP_REPLY))
		is_garp =
			...
			tha &&
			!memcmp(tha, sha, dev->addr_len);
    ...
}

The new is_valid_garp applies the tha == sha test to requests too.

The comment above says RFC 5227 Address Announcements require
tha == sha. RFC 5227 announcements are actually requests with THA zeroed.
They only get past this block because is_valid_ether_addr() is false for
an all-zero THA.

Everywhere else in arp.c, a sip == tip request with a unicast THA that
differs from sha counts as GARP. Here it is dropped as a "unicast THA ARP
Req" instead of deferring to DROP_GRATUITOUS_ARP, as the comment
describes. Is that intended?

> +
> +		if (arp->ar_op == htons(ARPOP_REPLY) && !is_valid_garp) {
> +			net_warn_ratelimited("Drop ARP Reply in L2 BMC on %s\n",
> +					     dev->name);
> +			goto out_free_skb;
> +		}

[Severity: Medium]
Does this drop legitimate solicited ARP replies that are sent as
link-layer broadcast?

Nothing here checks for an outstanding INCOMPLETE neighbour entry for
sip, so every non-GARP reply in a broadcast/multicast frame is discarded.
RFC 3927 requires IPv4 link-local (169.254/16) hosts to send all ARP
packets, replies included, as link-layer broadcast. A host with the sysctl
set may then fail to resolve those neighbours.

Further down, arp_process() handles broadcast replies on purpose:

	/* Broadcast replies and request packets
	   do not assert neighbour reachability.
	 */
	if (arp->ar_op != htons(ARPOP_REPLY) ||
	    skb->pkt_type != PACKET_HOST)
		state = NUD_STALE;

Once the sysctl is enabled, broadcast replies can no longer reach that
path. GARP replies whose THA is zero or broadcast instead of sha are
dropped too.

Before this patch the sysctl only affected IP packets, so existing users
will see their ARP behaviour change.

[Severity: Low]
The commit message says MIB counters are incremented for dropped frames.
Both new ARP drops end up here, though:

out_free_skb:
	kfree_skb(skb);
	return NET_RX_DROP;

That reports SKB_DROP_REASON_NOT_SPECIFIED and increments no counter. The
IPv4 and IPv6 paths use SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST. Could
these drops use kfree_skb_reason() with that reason for consistency?

> +		if (dev->addr_len == ETH_ALEN && tha &&
> +		    is_valid_ether_addr(tha) && !is_valid_garp) {
> +			net_warn_ratelimited("Drop unicast THA ARP Req in L2 BMC on %s\n",
> +					     dev->name);
> +			goto out_free_skb;
> +		}
> +	}

[Severity: High]
Can the ARP poisoning described in the commit message still happen with
the sysctl enabled?

For requests, THA plays no part in neighbour learning. Take a forged L2
broadcast ARP request with:

  - sip = the gateway
  - sha = the attacker's MAC
  - tip = a local address
  - THA = 00:00:00:00:00:00 (normal for real requests) or
    ff:ff:ff:ff:ff:ff

is_valid_ether_addr() is false for both THA values, so the frame gets
past this block and reaches:

		if (addr_type == RTN_LOCAL) {
			...
			if (!dont_send) {
				n = neigh_event_ns(tbl, sha, &sip, dev);

neigh_event_ns() then creates or updates the gateway's entry with the
attacker's MAC, using NEIGH_UPDATE_F_OVERRIDE.

Replies have a similar bypass. Take a reply with sip == tip == the gateway
and sha == tha == the attacker's MAC:

  - It satisfies is_valid_garp, so it skips both new checks.
  - With DROP_GRATUITOUS_ARP at its default of 0, it reaches arp_is_garp().
  - arp_is_garp() returns true for a unicast sip.

The result is:

		override = time_after(jiffies,
				      n->updated +
				      NEIGH_VAR(n->parms, LOCKTIME)) ||
			   is_garp;

and neigh_update() overwrites the existing gateway entry.

Gating on THA doesn't change what is learned from sha and sip. Does this
approach need rethinking, rather than more THA cases?

[ ... ]

> diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
> index 9860178752b8c..7d6d3e72e557b 100644
> --- a/net/ipv4/ip_input.c
> +++ b/net/ipv4/ip_input.c
> @@ -456,10 +456,37 @@ static int ip_rcv_finish_core(struct net *net,

[ ... ]

>  		if (in_dev &&
>  		    IN_DEV_ORCONF(in_dev, DROP_UNICAST_IN_L2_MULTICAST)) {
> -			drop_reason = SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST;
> -			goto drop;
> +			bool is_dhcp_resp = false;
> +
> +			iph = ip_hdr(skb);
> +			if (iph->protocol == IPPROTO_UDP &&
> +			    !ip_is_fragment(iph) &&
> +			    pskb_may_pull(skb, iph->ihl * 4 +
> +					       sizeof(struct udphdr))) {
> +				const struct udphdr *uh;
> +
> +				iph = ip_hdr(skb);
> +				uh = (const struct udphdr *)
> +					(skb_network_header(skb) +
> +					 iph->ihl * 4);
> +				if (uh->dest == htons(68))
> +					is_dhcp_resp = true;
> +			}

[Severity: Medium]
Does this exemption weaken the protection the sysctl is meant to provide?

Before this patch, every unicast IP packet in an L2 broadcast/multicast
frame was dropped when the sysctl was set. Now any non-fragmented UDP
datagram with destination port 68 is accepted. Source port, addresses and
payload are not checked.

So a forged DHCPACK, DHCPNAK or FORCERENEW, or anything else sent to port
68, injected in a GTK-encrypted broadcast, would reach a client that uses
a UDP socket bound to its leased address.

The commit message says this preserves DHCPOFFER/DHCPACK delivery. Is that
case actually reachable here?

Per RFC 2131 section 4.1, servers do one of two things:

  - IP-broadcast to 255.255.255.255, which routes as RTN_BROADCAST and
    never reaches this branch.
  - L2-unicast to chaddr/yiaddr, which arrives as PACKET_HOST.

In INIT/SELECTING the client doesn't own yiaddr yet, so
ip_route_input_slow() takes:

	if (!IN_DEV_FORWARD(in_dev)) {
		err = -EHOSTUNREACH;
		goto no_route;
	}

The OFFER then ends up in ip_error() rather than at a port 68 socket.

Clients that use AF_PACKET see the frame before IP input, so this drop
never affected them.

[Severity: Low]
ip6_rcv_core() gets no matching DHCPv6 (UDP port 546) exemption. The IPv4
and IPv6 drop_unicast_in_l2_multicast sysctls now behave differently,
while their ip-sysctl.rst entries are still worded the same.

If the IPv4 rationale holds, doesn't IPv6 need the same exemption? If it
doesn't hold, is the IPv4 exemption needed at all?

> +
> +			if (!is_dhcp_resp) {
> +				net_warn_ratelimited("Drop unicast IP %pI4 in L2 BMC on %s\n",
> +						     &iph->daddr, dev->name);

[Severity: High]
Can iph be stale here when pskb_may_pull() fails?

iph is only reloaded in the success branch. pskb_may_pull() can
reallocate skb->head and still return false:

include/linux/skbuff.h:pskb_may_pull_reason() {
	...
	skb_might_realloc(skb);

	if (likely(len <= skb_headlen(skb)))
		return SKB_NOT_DROPPED_YET;

	if (unlikely(len > skb->len))
		return SKB_DROP_REASON_PKT_TOO_SMALL;

	if (unlikely(!__pskb_pull_tail(skb, len - skb_headlen(skb))))
		return SKB_DROP_REASON_NOMEM;
	...
}

There are two ways to get there:

  - With CONFIG_FAIL_SKB_REALLOC, skb_might_realloc() calls
    pskb_expand_head(), which frees the old head of a non-cloned skb. A
    UDP packet whose tot_len equals the IP header length then returns
    PKT_TOO_SMALL.
  - Without fault injection, __pskb_pull_tail() can call
    pskb_expand_head() when eat > 0 or the skb is cloned. It can then
    still return NULL on the frag_list path, if skb_clone() or pskb_pull()
    fails.

Either way, is_dhcp_resp stays false and &iph->daddr is read through the
freed head for the %pI4 print.

The commit message says ip_hdr(skb) is reloaded after pskb_may_pull() to
prevent dangling pointer access, but that only covers the success path.
Would it work to reload iph unconditionally after the call, or to save
daddr before it?

[Severity: Low]
Are these new net_warn_ratelimited() calls needed? That means this one,
the one in ip6_rcv_core(), and the two in arp_process(). Any L2 peer can
trigger them at line rate with crafted frames.

net_ratelimit() uses a single global state shared by every
net_*_ratelimited() caller:

net/core/utils.c:
DEFINE_RATELIMIT_STATE(net_ratelimit_state, 5 * HZ, 10);

A steady flood would keep unrelated networking warnings suppressed, and
it would fill the log with attacker-chosen addresses.

The RFC 1122 text quoted just above this code says to silently discard.
These drops are also already visible through
SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST.

> +				__IP_INC_STATS(net, IPSTATS_MIB_INHDRERRORS);
> +				drop_reason =
> +					SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST;
> +				goto drop;
> +			}
>  		}
>  	}
>  
> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> index d332ec60f9150..816ee7874076b 100644
> --- a/net/ipv6/ip6_input.c
> +++ b/net/ipv6/ip6_input.c
> @@ -278,6 +278,8 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
>  	    (skb->pkt_type == PACKET_BROADCAST ||
>  	     skb->pkt_type == PACKET_MULTICAST) &&
>  	    READ_ONCE(idev->cnf.drop_unicast_in_l2_multicast)) {
> +		net_warn_ratelimited("IPv6: Drop unicast IP %pI6c in L2 BMC on %s\n",
> +				     &hdr->daddr, dev->name);
>  		SKB_DR_SET(reason, UNICAST_IN_L2_MULTICAST);
>  		goto err;
>  	}

[Severity: Low]
The commit message says:

    3. Dropped packets were silently discarded without warnings or MIB stats.

For IPv6 that doesn't look accurate. This drop already goes through the
err label:

err:
	__IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
	SKB_DR_OR(reason, IP_INHDR);

Both IPv4 and IPv6 also already reported
SKB_DROP_REASON_UNICAST_IN_L2_MULTICAST.

So the only IPv6 change is this printk, and it carries
Fixes: abbc30436d39, which makes it a stable candidate. Is that tag
appropriate for this hunk?

Two other points don't match the description either:

  - The new ARP drops get no counter at all.
  - The new IPv4 counter records a policy drop as
    IPSTATS_MIB_INHDRERRORS, even though the header is well-formed.

Could the commit message be updated to describe what actually changed?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261027015619.1745984-1-starchang%40google.com