* [PATCH nf,v5] netfilter: arp_tables: fix IEEE1394 ARP payload parsing
@ 2026-04-21 18:35 Pablo Neira Ayuso
2026-04-22 8:59 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Pablo Neira Ayuso @ 2026-04-21 18:35 UTC (permalink / raw)
To: netfilter-devel; +Cc: fw
Weiming Shi says:
"arp_packet_match() unconditionally parses the ARP payload assuming two
hardware addresses are present (source and target). However,
IPv4-over-IEEE1394 ARP (RFC 2734) omits the target hardware address
field, and arp_hdr_len() already accounts for this by returning a
shorter length for ARPHRD_IEEE1394 devices.
As a result, on IEEE1394 interfaces arp_packet_match() advances past a
nonexistent target hardware address and reads the wrong bytes for both
the target device address comparison and the target IP address. This
causes arptables rules to match against garbage data, leading to
incorrect filtering decisions: packets that should be accepted may be
dropped and vice versa.
The ARP stack in net/ipv4/arp.c (arp_create and arp_process) already
handles this correctly by skipping the target hardware address for
ARPHRD_IEEE1394. Apply the same pattern to arp_packet_match()."
This patch always returns 0 (no match) in case user matches on the target
hardware address which is never present in IEEE1394.
Note that this returns 0 (no match) for either normal and inverse match
because matching in the target hardware address in ARPHRD_IEEE1394 has
never been supported by arptables. This is intentional, matching on the
target hardware address should never evaluate true for ARPHRD_IEEE1394.
Moreover, adjust arpt_mangle to drop the packet if user tries to mangle
target hardware and IP address in IEEE1394, this has never been
supported.
Fixes: 6752c8db8e0c ("firewire net, ipv4 arp: Extend hardware address and remove driver-level packet inspection.")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v5: check for arphdr->ar_hrd == htons(ARPHRD_IEEE1394) in
arp_packet_match() too.
net/ipv4/netfilter/arp_tables.c | 19 ++++++++++++++++---
net/ipv4/netfilter/arpt_mangle.c | 8 ++++++++
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 1cdd9c28ab2d..e4b2106d0456 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -110,13 +110,26 @@ static inline int arp_packet_match(const struct arphdr *arphdr,
arpptr += dev->addr_len;
memcpy(&src_ipaddr, arpptr, sizeof(u32));
arpptr += sizeof(u32);
- tgt_devaddr = arpptr;
- arpptr += dev->addr_len;
+
+ if (IS_ENABLED(CONFIG_FIREWIRE_NET) &&
+ arphdr->ar_hrd == htons(ARPHRD_IEEE1394)) {
+ if (unlikely(memchr_inv(arpinfo->tgt_devaddr.mask, 0,
+ sizeof(arpinfo->tgt_devaddr.mask))))
+ return 0;
+
+ tgt_devaddr = NULL;
+ } else {
+ tgt_devaddr = arpptr;
+ arpptr += dev->addr_len;
+ }
memcpy(&tgt_ipaddr, arpptr, sizeof(u32));
if (NF_INVF(arpinfo, ARPT_INV_SRCDEVADDR,
arp_devaddr_compare(&arpinfo->src_devaddr, src_devaddr,
- dev->addr_len)) ||
+ dev->addr_len)))
+ return 0;
+
+ if (tgt_devaddr &&
NF_INVF(arpinfo, ARPT_INV_TGTDEVADDR,
arp_devaddr_compare(&arpinfo->tgt_devaddr, tgt_devaddr,
dev->addr_len)))
diff --git a/net/ipv4/netfilter/arpt_mangle.c b/net/ipv4/netfilter/arpt_mangle.c
index a4e07e5e9c11..285b1123b05c 100644
--- a/net/ipv4/netfilter/arpt_mangle.c
+++ b/net/ipv4/netfilter/arpt_mangle.c
@@ -40,6 +40,10 @@ target(struct sk_buff *skb, const struct xt_action_param *par)
}
arpptr += pln;
if (mangle->flags & ARPT_MANGLE_TDEV) {
+ if (IS_ENABLED(CONFIG_FIREWIRE_NET) &&
+ arp->ar_hrd == htons(ARPHRD_IEEE1394))
+ return NF_DROP;
+
if (ARPT_DEV_ADDR_LEN_MAX < hln ||
(arpptr + hln > skb_tail_pointer(skb)))
return NF_DROP;
@@ -47,6 +51,10 @@ target(struct sk_buff *skb, const struct xt_action_param *par)
}
arpptr += hln;
if (mangle->flags & ARPT_MANGLE_TIP) {
+ if (IS_ENABLED(CONFIG_FIREWIRE_NET) &&
+ arp->ar_hrd == htons(ARPHRD_IEEE1394))
+ return NF_DROP;
+
if (ARPT_MANGLE_ADDR_LEN_MAX < pln ||
(arpptr + pln > skb_tail_pointer(skb)))
return NF_DROP;
--
2.47.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH nf,v5] netfilter: arp_tables: fix IEEE1394 ARP payload parsing
2026-04-21 18:35 [PATCH nf,v5] netfilter: arp_tables: fix IEEE1394 ARP payload parsing Pablo Neira Ayuso
@ 2026-04-22 8:59 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2026-04-22 8:59 UTC (permalink / raw)
To: netfilter-devel; +Cc: fw
Hi Florian,
I am tossing this approach and getting back to the approach that
checks skb->dev, arp_process comes _after_ NF_HOOK_ARP_IN.
On Tue, Apr 21, 2026 at 08:35:14PM +0200, Pablo Neira Ayuso wrote:
> Weiming Shi says:
>
> "arp_packet_match() unconditionally parses the ARP payload assuming two
> hardware addresses are present (source and target). However,
> IPv4-over-IEEE1394 ARP (RFC 2734) omits the target hardware address
> field, and arp_hdr_len() already accounts for this by returning a
> shorter length for ARPHRD_IEEE1394 devices.
>
> As a result, on IEEE1394 interfaces arp_packet_match() advances past a
> nonexistent target hardware address and reads the wrong bytes for both
> the target device address comparison and the target IP address. This
> causes arptables rules to match against garbage data, leading to
> incorrect filtering decisions: packets that should be accepted may be
> dropped and vice versa.
>
> The ARP stack in net/ipv4/arp.c (arp_create and arp_process) already
> handles this correctly by skipping the target hardware address for
> ARPHRD_IEEE1394. Apply the same pattern to arp_packet_match()."
>
> This patch always returns 0 (no match) in case user matches on the target
> hardware address which is never present in IEEE1394.
>
> Note that this returns 0 (no match) for either normal and inverse match
> because matching in the target hardware address in ARPHRD_IEEE1394 has
> never been supported by arptables. This is intentional, matching on the
> target hardware address should never evaluate true for ARPHRD_IEEE1394.
>
> Moreover, adjust arpt_mangle to drop the packet if user tries to mangle
> target hardware and IP address in IEEE1394, this has never been
> supported.
>
> Fixes: 6752c8db8e0c ("firewire net, ipv4 arp: Extend hardware address and remove driver-level packet inspection.")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> v5: check for arphdr->ar_hrd == htons(ARPHRD_IEEE1394) in
> arp_packet_match() too.
>
> net/ipv4/netfilter/arp_tables.c | 19 ++++++++++++++++---
> net/ipv4/netfilter/arpt_mangle.c | 8 ++++++++
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
> index 1cdd9c28ab2d..e4b2106d0456 100644
> --- a/net/ipv4/netfilter/arp_tables.c
> +++ b/net/ipv4/netfilter/arp_tables.c
> @@ -110,13 +110,26 @@ static inline int arp_packet_match(const struct arphdr *arphdr,
> arpptr += dev->addr_len;
> memcpy(&src_ipaddr, arpptr, sizeof(u32));
> arpptr += sizeof(u32);
> - tgt_devaddr = arpptr;
> - arpptr += dev->addr_len;
> +
> + if (IS_ENABLED(CONFIG_FIREWIRE_NET) &&
> + arphdr->ar_hrd == htons(ARPHRD_IEEE1394)) {
> + if (unlikely(memchr_inv(arpinfo->tgt_devaddr.mask, 0,
> + sizeof(arpinfo->tgt_devaddr.mask))))
> + return 0;
> +
> + tgt_devaddr = NULL;
> + } else {
> + tgt_devaddr = arpptr;
> + arpptr += dev->addr_len;
> + }
> memcpy(&tgt_ipaddr, arpptr, sizeof(u32));
>
> if (NF_INVF(arpinfo, ARPT_INV_SRCDEVADDR,
> arp_devaddr_compare(&arpinfo->src_devaddr, src_devaddr,
> - dev->addr_len)) ||
> + dev->addr_len)))
> + return 0;
> +
> + if (tgt_devaddr &&
> NF_INVF(arpinfo, ARPT_INV_TGTDEVADDR,
> arp_devaddr_compare(&arpinfo->tgt_devaddr, tgt_devaddr,
> dev->addr_len)))
> diff --git a/net/ipv4/netfilter/arpt_mangle.c b/net/ipv4/netfilter/arpt_mangle.c
> index a4e07e5e9c11..285b1123b05c 100644
> --- a/net/ipv4/netfilter/arpt_mangle.c
> +++ b/net/ipv4/netfilter/arpt_mangle.c
> @@ -40,6 +40,10 @@ target(struct sk_buff *skb, const struct xt_action_param *par)
> }
> arpptr += pln;
> if (mangle->flags & ARPT_MANGLE_TDEV) {
> + if (IS_ENABLED(CONFIG_FIREWIRE_NET) &&
> + arp->ar_hrd == htons(ARPHRD_IEEE1394))
> + return NF_DROP;
> +
> if (ARPT_DEV_ADDR_LEN_MAX < hln ||
> (arpptr + hln > skb_tail_pointer(skb)))
> return NF_DROP;
> @@ -47,6 +51,10 @@ target(struct sk_buff *skb, const struct xt_action_param *par)
> }
> arpptr += hln;
> if (mangle->flags & ARPT_MANGLE_TIP) {
> + if (IS_ENABLED(CONFIG_FIREWIRE_NET) &&
> + arp->ar_hrd == htons(ARPHRD_IEEE1394))
> + return NF_DROP;
> +
> if (ARPT_MANGLE_ADDR_LEN_MAX < pln ||
> (arpptr + pln > skb_tail_pointer(skb)))
> return NF_DROP;
> --
> 2.47.3
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-04-22 9:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-21 18:35 [PATCH nf,v5] netfilter: arp_tables: fix IEEE1394 ARP payload parsing Pablo Neira Ayuso
2026-04-22 8:59 ` Pablo Neira Ayuso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.