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

[Julian Anastasov <ja@ssi.bg>]

	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>