[PATCH net 1/2] netfilter: nf_tables: set transport offset from mac header for netdev/egress

[PATCH net 1/2] netfilter: nf_tables: set transport offset from mac header for netdev/egress

[Pablo Neira Ayuso]

Before this patch, transport offset (pkt->thoff) provides an offset
relative to the network header. This is fine for the inet families
because skb->data points to the network header in such case. However,
from netdev/egress, skb->data points to the mac header (if available),
thus, pkt->thoff is missing the mac header length.

Add skb_network_offset() to the transport offset (pkt->thoff) for
netdev, so transport header mangling works as expected. Adjust payload
fast eval function to use skb->data now that pkt->thoff provides an
absolute offset. This explains why users report that matching on
egress/netdev works but payload mangling does not.

This patch implicitly fixes payload mangling for IPv4 packets in
netdev/egress given skb_store_bits() requires an offset from skb->data
to reach the transport header.

I suspect that nft_exthdr and the trace infra were also broken from
netdev/egress because they also take skb->data as start, and pkt->thoff
was not correct.

Note that IPv6 is fine because ipv6_find_hdr() already provides a
transport offset starting from skb->data, which includes
skb_network_offset().

The bridge family also uses nft_set_pktinfo_ipv4_validate(), but there
skb_network_offset() is zero, so the update in this patch does not alter
the existing behaviour.

Fixes: 42df6e1d221d ("netfilter: Introduce egress hook")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables_ipv4.h | 2 +-
 net/netfilter/nf_tables_core.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/netfilter/nf_tables_ipv4.h b/include/net/netfilter/nf_tables_ipv4.h
index 947973623dc7..60a7d0ce3080 100644
--- a/include/net/netfilter/nf_tables_ipv4.h
+++ b/include/net/netfilter/nf_tables_ipv4.h
@@ -30,7 +30,7 @@ static inline int __nft_set_pktinfo_ipv4_validate(struct nft_pktinfo *pkt)
 		return -1;
 
 	len = iph_totlen(pkt->skb, iph);
-	thoff = iph->ihl * 4;
+	thoff = skb_network_offset(pkt->skb) + (iph->ihl * 4);
 	if (pkt->skb->len < len)
 		return -1;
 	else if (len < thoff)
diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index 8b536d7ef6c2..c3e635364701 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -158,7 +158,7 @@ static bool nft_payload_fast_eval(const struct nft_expr *expr,
 	else {
 		if (!(pkt->flags & NFT_PKTINFO_L4PROTO))
 			return false;
-		ptr = skb_network_header(skb) + nft_thoff(pkt);
+		ptr = skb->data + nft_thoff(pkt);
 	}
 
 	ptr += priv->offset;
-- 
2.30.2

[PATCH net 0/2] Netfilter fixes for net

[Pablo Neira Ayuso]

[ resent, apparently this was only posted to netfilter-devel@vger.kernel.org,
  not to netdev@vger.kernel.org ]

Hi,

The following patchset contains Netfilter fixes for net:

1) Skip set commit for deleted/destroyed sets, this might trigger
   double deactivation of expired elements.

2) Fix packet mangling from egress, set transport offset from
   mac header for netdev/egress.

Both fixes address bugs already present in several releases.

Please, pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-23-12-20

Thanks.

----------------------------------------------------------------

The following changes since commit 8353c2abc02cf8302d5e6177b706c1879e7b833c:

  Merge branch 'check-vlan-filter-feature-in-vlan_vids_add_by_dev-and-vlan_vids_del_by_dev' (2023-12-19 13:13:59 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git tags/nf-23-12-20

for you to fetch changes up to 7315dc1e122c85ffdfc8defffbb8f8b616c2eb1a:

  netfilter: nf_tables: skip set commit for deleted/destroyed sets (2023-12-20 13:48:00 +0100)

----------------------------------------------------------------
netfilter pull request 23-12-20

----------------------------------------------------------------
Pablo Neira Ayuso (2):
      netfilter: nf_tables: set transport offset from mac header for netdev/egress
      netfilter: nf_tables: skip set commit for deleted/destroyed sets

 include/net/netfilter/nf_tables_ipv4.h | 2 +-
 net/netfilter/nf_tables_api.c          | 2 +-
 net/netfilter/nf_tables_core.c         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

[PATCH net 1/2] netfilter: nf_tables: set transport offset from mac header for netdev/egress

[Pablo Neira Ayuso]

Before this patch, transport offset (pkt->thoff) provides an offset
relative to the network header. This is fine for the inet families
because skb->data points to the network header in such case. However,
from netdev/egress, skb->data points to the mac header (if available),
thus, pkt->thoff is missing the mac header length.

Add skb_network_offset() to the transport offset (pkt->thoff) for
netdev, so transport header mangling works as expected. Adjust payload
fast eval function to use skb->data now that pkt->thoff provides an
absolute offset. This explains why users report that matching on
egress/netdev works but payload mangling does not.

This patch implicitly fixes payload mangling for IPv4 packets in
netdev/egress given skb_store_bits() requires an offset from skb->data
to reach the transport header.

I suspect that nft_exthdr and the trace infra were also broken from
netdev/egress because they also take skb->data as start, and pkt->thoff
was not correct.

Note that IPv6 is fine because ipv6_find_hdr() already provides a
transport offset starting from skb->data, which includes
skb_network_offset().

The bridge family also uses nft_set_pktinfo_ipv4_validate(), but there
skb_network_offset() is zero, so the update in this patch does not alter
the existing behaviour.

Fixes: 42df6e1d221d ("netfilter: Introduce egress hook")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables_ipv4.h | 2 +-
 net/netfilter/nf_tables_core.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/netfilter/nf_tables_ipv4.h b/include/net/netfilter/nf_tables_ipv4.h
index 947973623dc7..60a7d0ce3080 100644
--- a/include/net/netfilter/nf_tables_ipv4.h
+++ b/include/net/netfilter/nf_tables_ipv4.h
@@ -30,7 +30,7 @@ static inline int __nft_set_pktinfo_ipv4_validate(struct nft_pktinfo *pkt)
 		return -1;
 
 	len = iph_totlen(pkt->skb, iph);
-	thoff = iph->ihl * 4;
+	thoff = skb_network_offset(pkt->skb) + (iph->ihl * 4);
 	if (pkt->skb->len < len)
 		return -1;
 	else if (len < thoff)
diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index 8b536d7ef6c2..c3e635364701 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -158,7 +158,7 @@ static bool nft_payload_fast_eval(const struct nft_expr *expr,
 	else {
 		if (!(pkt->flags & NFT_PKTINFO_L4PROTO))
 			return false;
-		ptr = skb_network_header(skb) + nft_thoff(pkt);
+		ptr = skb->data + nft_thoff(pkt);
 	}
 
 	ptr += priv->offset;
-- 
2.30.2

[PATCH net 2/2] netfilter: nf_tables: skip set commit for deleted/destroyed sets

[Pablo Neira Ayuso]

NFT_MSG_DELSET deactivates all elements in the set, skip
set->ops->commit() to avoid the unnecessary clone (for the pipapo case)
as well as the sync GC cycle, which could deactivate again expired
elements in such set.

Fixes: 5f68718b34a5 ("netfilter: nf_tables: GC transaction API to avoid race with control plane")
Reported-by: Kevin Rich <kevinrich1337@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c5c17c6e80ed..be04af433988 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -9887,7 +9887,7 @@ static void nft_set_commit_update(struct list_head *set_update_list)
 	list_for_each_entry_safe(set, next, set_update_list, pending_update) {
 		list_del_init(&set->pending_update);
 
-		if (!set->ops->commit)
+		if (!set->ops->commit || set->dead)
 			continue;
 
 		set->ops->commit(set);
-- 
2.30.2

Re: [PATCH net 0/2] Netfilter fixes for net

[Pablo Neira Ayuso]

On Fri, Dec 22, 2023 at 11:42:03AM +0100, Pablo Neira Ayuso wrote:
> [ resent, apparently this was only posted to netfilter-devel@vger.kernel.org,
>   not to netdev@vger.kernel.org ]

For the record, previous is still in patchwork:

https://patchwork.kernel.org/project/netdevbpf/patch/20231220151544.270214-1-pablo@netfilter.org/

> Hi,
> 
> The following patchset contains Netfilter fixes for net:
> 
> 1) Skip set commit for deleted/destroyed sets, this might trigger
>    double deactivation of expired elements.
> 
> 2) Fix packet mangling from egress, set transport offset from
>    mac header for netdev/egress.
> 
> Both fixes address bugs already present in several releases.
> 
> Please, pull these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-23-12-20
> 
> Thanks.
> 
> ----------------------------------------------------------------
> 
> The following changes since commit 8353c2abc02cf8302d5e6177b706c1879e7b833c:
> 
>   Merge branch 'check-vlan-filter-feature-in-vlan_vids_add_by_dev-and-vlan_vids_del_by_dev' (2023-12-19 13:13:59 +0100)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git tags/nf-23-12-20
> 
> for you to fetch changes up to 7315dc1e122c85ffdfc8defffbb8f8b616c2eb1a:
> 
>   netfilter: nf_tables: skip set commit for deleted/destroyed sets (2023-12-20 13:48:00 +0100)
> 
> ----------------------------------------------------------------
> netfilter pull request 23-12-20
> 
> ----------------------------------------------------------------
> Pablo Neira Ayuso (2):
>       netfilter: nf_tables: set transport offset from mac header for netdev/egress
>       netfilter: nf_tables: skip set commit for deleted/destroyed sets
> 
>  include/net/netfilter/nf_tables_ipv4.h | 2 +-
>  net/netfilter/nf_tables_api.c          | 2 +-
>  net/netfilter/nf_tables_core.c         | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 

Re: [PATCH net 0/2] Netfilter fixes for net

[Pablo Neira Ayuso]

On Fri, Dec 22, 2023 at 11:49:14AM +0100, Pablo Neira Ayuso wrote:
> On Fri, Dec 22, 2023 at 11:42:03AM +0100, Pablo Neira Ayuso wrote:
> > [ resent, apparently this was only posted to netfilter-devel@vger.kernel.org,
> >   not to netdev@vger.kernel.org ]
> 
> For the record, previous is still in patchwork:
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/20231220151544.270214-1-pablo@netfilter.org/

Actually, I Cc'ed netdev@vger.kernel.org in PR from Dec 20.

So this PR got lost?

Re: [PATCH net 1/2] netfilter: nf_tables: set transport offset from mac header for netdev/egress

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Pablo Neira Ayuso <pablo@netfilter.org>:

On Fri, 22 Dec 2023 11:42:04 +0100 you wrote:
> Before this patch, transport offset (pkt->thoff) provides an offset
> relative to the network header. This is fine for the inet families
> because skb->data points to the network header in such case. However,
> from netdev/egress, skb->data points to the mac header (if available),
> thus, pkt->thoff is missing the mac header length.
> 
> Add skb_network_offset() to the transport offset (pkt->thoff) for
> netdev, so transport header mangling works as expected. Adjust payload
> fast eval function to use skb->data now that pkt->thoff provides an
> absolute offset. This explains why users report that matching on
> egress/netdev works but payload mangling does not.
> 
> [...]

Here is the summary with links:
  - [net,1/2] netfilter: nf_tables: set transport offset from mac header for netdev/egress
    https://git.kernel.org/netdev/net/c/0ae8e4cca787
  - [net,2/2] netfilter: nf_tables: skip set commit for deleted/destroyed sets
    https://git.kernel.org/netdev/net/c/7315dc1e122c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html