* [PATCH net-next 1/7] netfilter: xt_mangle: only check verdict part of return value
2023-10-18 8:51 [PATCH net-next 0/7] netfilter updates for net-next Florian Westphal
@ 2023-10-18 8:51 ` Florian Westphal
2023-10-18 10:10 ` patchwork-bot+netdevbpf
2023-10-18 8:51 ` [PATCH net-next 2/7] netfilter: nf_tables: mask out non-verdict bits when checking " Florian Westphal
` (5 subsequent siblings)
6 siblings, 1 reply; 9+ messages in thread
From: Florian Westphal @ 2023-10-18 8:51 UTC (permalink / raw)
To: netdev
Cc: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
netfilter-devel
These checks assume that the caller only returns NF_DROP without
any errno embedded in the upper bits.
This is fine right now, but followup patches will start to propagate
such errors to allow kfree_skb_drop_reason() in the called functions,
those would then indicate 'errno << 8 | NF_STOLEN'.
To not break things we have to mask those parts out.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/ipv4/netfilter/iptable_mangle.c | 9 +++++----
net/ipv6/netfilter/ip6table_mangle.c | 9 +++++----
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/netfilter/iptable_mangle.c b/net/ipv4/netfilter/iptable_mangle.c
index 3abb430af9e6..385d945d8ebe 100644
--- a/net/ipv4/netfilter/iptable_mangle.c
+++ b/net/ipv4/netfilter/iptable_mangle.c
@@ -36,12 +36,12 @@ static const struct xt_table packet_mangler = {
static unsigned int
ipt_mangle_out(void *priv, struct sk_buff *skb, const struct nf_hook_state *state)
{
- unsigned int ret;
+ unsigned int ret, verdict;
const struct iphdr *iph;
- u_int8_t tos;
__be32 saddr, daddr;
- u_int32_t mark;
+ u32 mark;
int err;
+ u8 tos;
/* Save things which could affect route */
mark = skb->mark;
@@ -51,8 +51,9 @@ ipt_mangle_out(void *priv, struct sk_buff *skb, const struct nf_hook_state *stat
tos = iph->tos;
ret = ipt_do_table(priv, skb, state);
+ verdict = ret & NF_VERDICT_MASK;
/* Reroute for ANY change. */
- if (ret != NF_DROP && ret != NF_STOLEN) {
+ if (verdict != NF_DROP && verdict != NF_STOLEN) {
iph = ip_hdr(skb);
if (iph->saddr != saddr ||
diff --git a/net/ipv6/netfilter/ip6table_mangle.c b/net/ipv6/netfilter/ip6table_mangle.c
index a88b2ce4a3cb..8dd4cd0c47bd 100644
--- a/net/ipv6/netfilter/ip6table_mangle.c
+++ b/net/ipv6/netfilter/ip6table_mangle.c
@@ -31,10 +31,10 @@ static const struct xt_table packet_mangler = {
static unsigned int
ip6t_mangle_out(void *priv, struct sk_buff *skb, const struct nf_hook_state *state)
{
- unsigned int ret;
struct in6_addr saddr, daddr;
- u_int8_t hop_limit;
- u_int32_t flowlabel, mark;
+ unsigned int ret, verdict;
+ u32 flowlabel, mark;
+ u8 hop_limit;
int err;
/* save source/dest address, mark, hoplimit, flowlabel, priority, */
@@ -47,8 +47,9 @@ ip6t_mangle_out(void *priv, struct sk_buff *skb, const struct nf_hook_state *sta
flowlabel = *((u_int32_t *)ipv6_hdr(skb));
ret = ip6t_do_table(priv, skb, state);
+ verdict = ret & NF_VERDICT_MASK;
- if (ret != NF_DROP && ret != NF_STOLEN &&
+ if (verdict != NF_DROP && verdict != NF_STOLEN &&
(!ipv6_addr_equal(&ipv6_hdr(skb)->saddr, &saddr) ||
!ipv6_addr_equal(&ipv6_hdr(skb)->daddr, &daddr) ||
skb->mark != mark ||
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/7] netfilter: xt_mangle: only check verdict part of return value
2023-10-18 8:51 ` [PATCH net-next 1/7] netfilter: xt_mangle: only check verdict part of return value Florian Westphal
@ 2023-10-18 10:10 ` patchwork-bot+netdevbpf
0 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-10-18 10:10 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev, pabeni, davem, edumazet, kuba, netfilter-devel
Hello:
This series was applied to netdev/net-next.git (main)
by Florian Westphal <fw@strlen.de>:
On Wed, 18 Oct 2023 10:51:05 +0200 you wrote:
> These checks assume that the caller only returns NF_DROP without
> any errno embedded in the upper bits.
>
> This is fine right now, but followup patches will start to propagate
> such errors to allow kfree_skb_drop_reason() in the called functions,
> those would then indicate 'errno << 8 | NF_STOLEN'.
>
> [...]
Here is the summary with links:
- [net-next,1/7] netfilter: xt_mangle: only check verdict part of return value
https://git.kernel.org/netdev/net-next/c/e15e5027106f
- [net-next,2/7] netfilter: nf_tables: mask out non-verdict bits when checking return value
https://git.kernel.org/netdev/net-next/c/4d26ab0086aa
- [net-next,3/7] netfilter: conntrack: convert nf_conntrack_update to netfilter verdicts
https://git.kernel.org/netdev/net-next/c/6291b3a67ad5
- [net-next,4/7] netfilter: nf_nat: mask out non-verdict bits when checking return value
https://git.kernel.org/netdev/net-next/c/35c038b0a4be
- [net-next,5/7] netfilter: make nftables drops visible in net dropmonitor
https://git.kernel.org/netdev/net-next/c/e0d4593140b0
- [net-next,6/7] netfilter: bridge: convert br_netfilter to NF_DROP_REASON
https://git.kernel.org/netdev/net-next/c/cf8b7c1a5be7
- [net-next,7/7] netfilter: nf_tables: de-constify set commit ops function argument
https://git.kernel.org/netdev/net-next/c/256001672153
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next 2/7] netfilter: nf_tables: mask out non-verdict bits when checking return value
2023-10-18 8:51 [PATCH net-next 0/7] netfilter updates for net-next Florian Westphal
2023-10-18 8:51 ` [PATCH net-next 1/7] netfilter: xt_mangle: only check verdict part of return value Florian Westphal
@ 2023-10-18 8:51 ` Florian Westphal
2023-10-18 8:51 ` [PATCH net-next 3/7] netfilter: conntrack: convert nf_conntrack_update to netfilter verdicts Florian Westphal
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Florian Westphal @ 2023-10-18 8:51 UTC (permalink / raw)
To: netdev
Cc: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
netfilter-devel
nftables trace infra must mask out the non-verdict bit parts of the
return value, else followup changes that 'return errno << 8 | NF_STOLEN'
will cause breakage.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_tables_core.c | 2 +-
net/netfilter/nf_tables_trace.c | 8 ++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index 4d0ce12221f6..6009b423f60a 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -115,7 +115,7 @@ static noinline void __nft_trace_verdict(const struct nft_pktinfo *pkt,
{
enum nft_trace_types type;
- switch (regs->verdict.code) {
+ switch (regs->verdict.code & NF_VERDICT_MASK) {
case NFT_CONTINUE:
case NFT_RETURN:
type = NFT_TRACETYPE_RETURN;
diff --git a/net/netfilter/nf_tables_trace.c b/net/netfilter/nf_tables_trace.c
index 6d41c0bd3d78..a83637e3f455 100644
--- a/net/netfilter/nf_tables_trace.c
+++ b/net/netfilter/nf_tables_trace.c
@@ -258,17 +258,21 @@ void nft_trace_notify(const struct nft_pktinfo *pkt,
case __NFT_TRACETYPE_MAX:
break;
case NFT_TRACETYPE_RETURN:
- case NFT_TRACETYPE_RULE:
+ case NFT_TRACETYPE_RULE: {
+ unsigned int v;
+
if (nft_verdict_dump(skb, NFTA_TRACE_VERDICT, verdict))
goto nla_put_failure;
/* pkt->skb undefined iff NF_STOLEN, disable dump */
- if (verdict->code == NF_STOLEN)
+ v = verdict->code & NF_VERDICT_MASK;
+ if (v == NF_STOLEN)
info->packet_dumped = true;
else
mark = pkt->skb->mark;
break;
+ }
case NFT_TRACETYPE_POLICY:
mark = pkt->skb->mark;
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 3/7] netfilter: conntrack: convert nf_conntrack_update to netfilter verdicts
2023-10-18 8:51 [PATCH net-next 0/7] netfilter updates for net-next Florian Westphal
2023-10-18 8:51 ` [PATCH net-next 1/7] netfilter: xt_mangle: only check verdict part of return value Florian Westphal
2023-10-18 8:51 ` [PATCH net-next 2/7] netfilter: nf_tables: mask out non-verdict bits when checking " Florian Westphal
@ 2023-10-18 8:51 ` Florian Westphal
2023-10-18 8:51 ` [PATCH net-next 4/7] netfilter: nf_nat: mask out non-verdict bits when checking return value Florian Westphal
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Florian Westphal @ 2023-10-18 8:51 UTC (permalink / raw)
To: netdev
Cc: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
netfilter-devel
This function calls helpers that can return nf-verdicts, but then
those get converted to -1/0 as thats what the caller expects.
Theoretically NF_DROP could have an errno number set in the upper 24
bits of the return value. Or any of those helpers could return
NF_STOLEN, which would result in use-after-free.
This is fine as-is, the called functions don't do this yet.
But its better to avoid possible future problems if the upcoming
patchset to add NF_DROP_REASON() support gains further users, so remove
the 0/-1 translation from the picture and pass the verdicts down to
the caller.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_conntrack_core.c | 58 ++++++++++++++++++-------------
net/netfilter/nfnetlink_queue.c | 15 ++++----
2 files changed, 42 insertions(+), 31 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 124136b5a79a..2e5f3864d353 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -2169,11 +2169,11 @@ static int __nf_conntrack_update(struct net *net, struct sk_buff *skb,
dataoff = get_l4proto(skb, skb_network_offset(skb), l3num, &l4num);
if (dataoff <= 0)
- return -1;
+ return NF_DROP;
if (!nf_ct_get_tuple(skb, skb_network_offset(skb), dataoff, l3num,
l4num, net, &tuple))
- return -1;
+ return NF_DROP;
if (ct->status & IPS_SRC_NAT) {
memcpy(tuple.src.u3.all,
@@ -2193,7 +2193,7 @@ static int __nf_conntrack_update(struct net *net, struct sk_buff *skb,
h = nf_conntrack_find_get(net, nf_ct_zone(ct), &tuple);
if (!h)
- return 0;
+ return NF_ACCEPT;
/* Store status bits of the conntrack that is clashing to re-do NAT
* mangling according to what it has been done already to this packet.
@@ -2206,19 +2206,25 @@ static int __nf_conntrack_update(struct net *net, struct sk_buff *skb,
nat_hook = rcu_dereference(nf_nat_hook);
if (!nat_hook)
- return 0;
+ return NF_ACCEPT;
- if (status & IPS_SRC_NAT &&
- nat_hook->manip_pkt(skb, ct, NF_NAT_MANIP_SRC,
- IP_CT_DIR_ORIGINAL) == NF_DROP)
- return -1;
+ if (status & IPS_SRC_NAT) {
+ unsigned int verdict = nat_hook->manip_pkt(skb, ct,
+ NF_NAT_MANIP_SRC,
+ IP_CT_DIR_ORIGINAL);
+ if (verdict != NF_ACCEPT)
+ return verdict;
+ }
- if (status & IPS_DST_NAT &&
- nat_hook->manip_pkt(skb, ct, NF_NAT_MANIP_DST,
- IP_CT_DIR_ORIGINAL) == NF_DROP)
- return -1;
+ if (status & IPS_DST_NAT) {
+ unsigned int verdict = nat_hook->manip_pkt(skb, ct,
+ NF_NAT_MANIP_DST,
+ IP_CT_DIR_ORIGINAL);
+ if (verdict != NF_ACCEPT)
+ return verdict;
+ }
- return 0;
+ return NF_ACCEPT;
}
/* This packet is coming from userspace via nf_queue, complete the packet
@@ -2233,14 +2239,14 @@ static int nf_confirm_cthelper(struct sk_buff *skb, struct nf_conn *ct,
help = nfct_help(ct);
if (!help)
- return 0;
+ return NF_ACCEPT;
helper = rcu_dereference(help->helper);
if (!helper)
- return 0;
+ return NF_ACCEPT;
if (!(helper->flags & NF_CT_HELPER_F_USERSPACE))
- return 0;
+ return NF_ACCEPT;
switch (nf_ct_l3num(ct)) {
case NFPROTO_IPV4:
@@ -2255,42 +2261,44 @@ static int nf_confirm_cthelper(struct sk_buff *skb, struct nf_conn *ct,
protoff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &pnum,
&frag_off);
if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
- return 0;
+ return NF_ACCEPT;
break;
}
#endif
default:
- return 0;
+ return NF_ACCEPT;
}
if (test_bit(IPS_SEQ_ADJUST_BIT, &ct->status) &&
!nf_is_loopback_packet(skb)) {
if (!nf_ct_seq_adjust(skb, ct, ctinfo, protoff)) {
NF_CT_STAT_INC_ATOMIC(nf_ct_net(ct), drop);
- return -1;
+ return NF_DROP;
}
}
/* We've seen it coming out the other side: confirm it */
- return nf_conntrack_confirm(skb) == NF_DROP ? - 1 : 0;
+ return nf_conntrack_confirm(skb);
}
static int nf_conntrack_update(struct net *net, struct sk_buff *skb)
{
enum ip_conntrack_info ctinfo;
struct nf_conn *ct;
- int err;
ct = nf_ct_get(skb, &ctinfo);
if (!ct)
- return 0;
+ return NF_ACCEPT;
if (!nf_ct_is_confirmed(ct)) {
- err = __nf_conntrack_update(net, skb, ct, ctinfo);
- if (err < 0)
- return err;
+ int ret = __nf_conntrack_update(net, skb, ct, ctinfo);
+
+ if (ret != NF_ACCEPT)
+ return ret;
ct = nf_ct_get(skb, &ctinfo);
+ if (!ct)
+ return NF_ACCEPT;
}
return nf_confirm_cthelper(skb, ct, ctinfo);
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 556bc902af00..171d1f52d3dd 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -228,19 +228,22 @@ find_dequeue_entry(struct nfqnl_instance *queue, unsigned int id)
static void nfqnl_reinject(struct nf_queue_entry *entry, unsigned int verdict)
{
const struct nf_ct_hook *ct_hook;
- int err;
if (verdict == NF_ACCEPT ||
verdict == NF_REPEAT ||
verdict == NF_STOP) {
rcu_read_lock();
ct_hook = rcu_dereference(nf_ct_hook);
- if (ct_hook) {
- err = ct_hook->update(entry->state.net, entry->skb);
- if (err < 0)
- verdict = NF_DROP;
- }
+ if (ct_hook)
+ verdict = ct_hook->update(entry->state.net, entry->skb);
rcu_read_unlock();
+
+ switch (verdict & NF_VERDICT_MASK) {
+ case NF_STOLEN:
+ nf_queue_entry_free(entry);
+ return;
+ }
+
}
nf_reinject(entry, verdict);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 4/7] netfilter: nf_nat: mask out non-verdict bits when checking return value
2023-10-18 8:51 [PATCH net-next 0/7] netfilter updates for net-next Florian Westphal
` (2 preceding siblings ...)
2023-10-18 8:51 ` [PATCH net-next 3/7] netfilter: conntrack: convert nf_conntrack_update to netfilter verdicts Florian Westphal
@ 2023-10-18 8:51 ` Florian Westphal
2023-10-18 8:51 ` [PATCH net-next 5/7] netfilter: make nftables drops visible in net dropmonitor Florian Westphal
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Florian Westphal @ 2023-10-18 8:51 UTC (permalink / raw)
To: netdev
Cc: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
netfilter-devel
Same as previous change: we need to mask out the non-verdict bits, as
upcoming patches may embed an errno value in NF_STOLEN verdicts too.
NF_DROP could already do this, but not all called functions do this.
Checks that only test ret vs NF_ACCEPT are fine, the 'errno parts'
are always 0 for those.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_nat_proto.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_nat_proto.c b/net/netfilter/nf_nat_proto.c
index 5a049740758f..6d969468c779 100644
--- a/net/netfilter/nf_nat_proto.c
+++ b/net/netfilter/nf_nat_proto.c
@@ -999,11 +999,12 @@ static unsigned int
nf_nat_ipv6_in(void *priv, struct sk_buff *skb,
const struct nf_hook_state *state)
{
- unsigned int ret;
+ unsigned int ret, verdict;
struct in6_addr daddr = ipv6_hdr(skb)->daddr;
ret = nf_nat_ipv6_fn(priv, skb, state);
- if (ret != NF_DROP && ret != NF_STOLEN &&
+ verdict = ret & NF_VERDICT_MASK;
+ if (verdict != NF_DROP && verdict != NF_STOLEN &&
ipv6_addr_cmp(&daddr, &ipv6_hdr(skb)->daddr))
skb_dst_drop(skb);
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 5/7] netfilter: make nftables drops visible in net dropmonitor
2023-10-18 8:51 [PATCH net-next 0/7] netfilter updates for net-next Florian Westphal
` (3 preceding siblings ...)
2023-10-18 8:51 ` [PATCH net-next 4/7] netfilter: nf_nat: mask out non-verdict bits when checking return value Florian Westphal
@ 2023-10-18 8:51 ` Florian Westphal
2023-10-18 8:51 ` [PATCH net-next 6/7] netfilter: bridge: convert br_netfilter to NF_DROP_REASON Florian Westphal
2023-10-18 8:51 ` [PATCH net-next 7/7] netfilter: nf_tables: de-constify set commit ops function argument Florian Westphal
6 siblings, 0 replies; 9+ messages in thread
From: Florian Westphal @ 2023-10-18 8:51 UTC (permalink / raw)
To: netdev
Cc: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
netfilter-devel
net_dropmonitor blames core.c:nf_hook_slow.
Add NF_DROP_REASON() helper and use it in nft_do_chain().
The helper releases the skb, so exact drop location becomes
available. Calling code will observe the NF_STOLEN verdict
instead.
Adjust nf_hook_slow so we can embed an erro value wih
NF_STOLEN verdicts, just like we do for NF_DROP.
After this, drop in nftables can be pinpointed to a drop due
to a rule or the chain policy.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/linux/netfilter.h | 10 ++++++++++
net/netfilter/core.c | 6 +++---
net/netfilter/nf_tables_core.c | 6 +++++-
3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index d68644b7c299..80900d910992 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -22,6 +22,16 @@ static inline int NF_DROP_GETERR(int verdict)
return -(verdict >> NF_VERDICT_QBITS);
}
+static __always_inline int
+NF_DROP_REASON(struct sk_buff *skb, enum skb_drop_reason reason, u32 err)
+{
+ BUILD_BUG_ON(err > 0xffff);
+
+ kfree_skb_reason(skb, reason);
+
+ return ((err << 16) | NF_STOLEN);
+}
+
static inline int nf_inet_addr_cmp(const union nf_inet_addr *a1,
const union nf_inet_addr *a2)
{
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index ef4e76e5aef9..3126911f5042 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -639,10 +639,10 @@ int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state,
if (ret == 1)
continue;
return ret;
+ case NF_STOLEN:
+ return NF_DROP_GETERR(verdict);
default:
- /* Implicit handling for NF_STOLEN, as well as any other
- * non conventional verdicts.
- */
+ WARN_ON_ONCE(1);
return 0;
}
}
diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index 6009b423f60a..8b536d7ef6c2 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -308,10 +308,11 @@ nft_do_chain(struct nft_pktinfo *pkt, void *priv)
switch (regs.verdict.code & NF_VERDICT_MASK) {
case NF_ACCEPT:
- case NF_DROP:
case NF_QUEUE:
case NF_STOLEN:
return regs.verdict.code;
+ case NF_DROP:
+ return NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
switch (regs.verdict.code) {
@@ -342,6 +343,9 @@ nft_do_chain(struct nft_pktinfo *pkt, void *priv)
if (static_branch_unlikely(&nft_counters_enabled))
nft_update_chain_stats(basechain, pkt);
+ if (nft_base_chain(basechain)->policy == NF_DROP)
+ return NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
+
return nft_base_chain(basechain)->policy;
}
EXPORT_SYMBOL_GPL(nft_do_chain);
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 6/7] netfilter: bridge: convert br_netfilter to NF_DROP_REASON
2023-10-18 8:51 [PATCH net-next 0/7] netfilter updates for net-next Florian Westphal
` (4 preceding siblings ...)
2023-10-18 8:51 ` [PATCH net-next 5/7] netfilter: make nftables drops visible in net dropmonitor Florian Westphal
@ 2023-10-18 8:51 ` Florian Westphal
2023-10-18 8:51 ` [PATCH net-next 7/7] netfilter: nf_tables: de-constify set commit ops function argument Florian Westphal
6 siblings, 0 replies; 9+ messages in thread
From: Florian Westphal @ 2023-10-18 8:51 UTC (permalink / raw)
To: netdev
Cc: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
netfilter-devel
errno is 0 because these hooks are called from prerouting and forward.
There is no socket that the errno would ever be propagated to.
Other netfilter modules (e.g. nf_nat, conntrack, ...) can be converted
in a similar way.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/bridge/br_netfilter_hooks.c | 26 +++++++++++++-------------
net/bridge/br_netfilter_ipv6.c | 6 +++---
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index 033034d68f1f..4c0c9f838f5c 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -486,11 +486,11 @@ static unsigned int br_nf_pre_routing(void *priv,
struct brnf_net *brnet;
if (unlikely(!pskb_may_pull(skb, len)))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_PKT_TOO_SMALL, 0);
p = br_port_get_rcu(state->in);
if (p == NULL)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_DEV_READY, 0);
br = p->br;
brnet = net_generic(state->net, brnf_net_id);
@@ -501,7 +501,7 @@ static unsigned int br_nf_pre_routing(void *priv,
return NF_ACCEPT;
if (!ipv6_mod_enabled()) {
pr_warn_once("Module ipv6 is disabled, so call_ip6tables is not supported.");
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_IPV6DISABLED, 0);
}
nf_bridge_pull_encap_header_rcsum(skb);
@@ -518,12 +518,12 @@ static unsigned int br_nf_pre_routing(void *priv,
nf_bridge_pull_encap_header_rcsum(skb);
if (br_validate_ipv4(state->net, skb))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_IP_INHDR, 0);
if (!nf_bridge_alloc(skb))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NOMEM, 0);
if (!setup_pre_routing(skb, state->net))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_DEV_READY, 0);
nf_bridge = nf_bridge_info_get(skb);
nf_bridge->ipv4_daddr = ip_hdr(skb)->daddr;
@@ -590,15 +590,15 @@ static unsigned int br_nf_forward_ip(void *priv,
/* Need exclusive nf_bridge_info since we might have multiple
* different physoutdevs. */
if (!nf_bridge_unshare(skb))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NOMEM, 0);
nf_bridge = nf_bridge_info_get(skb);
if (!nf_bridge)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NOMEM, 0);
parent = bridge_parent(state->out);
if (!parent)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_DEV_READY, 0);
if (IS_IP(skb) || is_vlan_ip(skb, state->net) ||
is_pppoe_ip(skb, state->net))
@@ -618,13 +618,13 @@ static unsigned int br_nf_forward_ip(void *priv,
if (pf == NFPROTO_IPV4) {
if (br_validate_ipv4(state->net, skb))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_IP_INHDR, 0);
IPCB(skb)->frag_max_size = nf_bridge->frag_max_size;
}
if (pf == NFPROTO_IPV6) {
if (br_validate_ipv6(state->net, skb))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_IP_INHDR, 0);
IP6CB(skb)->frag_max_size = nf_bridge->frag_max_size;
}
@@ -666,7 +666,7 @@ static unsigned int br_nf_forward_arp(void *priv,
}
if (unlikely(!pskb_may_pull(skb, sizeof(struct arphdr))))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_PKT_TOO_SMALL, 0);
if (arp_hdr(skb)->ar_pln != 4) {
if (is_vlan_arp(skb, state->net))
@@ -831,7 +831,7 @@ static unsigned int br_nf_post_routing(void *priv,
return NF_ACCEPT;
if (!realoutdev)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_DEV_READY, 0);
if (IS_IP(skb) || is_vlan_ip(skb, state->net) ||
is_pppoe_ip(skb, state->net))
diff --git a/net/bridge/br_netfilter_ipv6.c b/net/bridge/br_netfilter_ipv6.c
index 550039dfc31a..2e24a743f917 100644
--- a/net/bridge/br_netfilter_ipv6.c
+++ b/net/bridge/br_netfilter_ipv6.c
@@ -161,13 +161,13 @@ unsigned int br_nf_pre_routing_ipv6(void *priv,
struct nf_bridge_info *nf_bridge;
if (br_validate_ipv6(state->net, skb))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_IP_INHDR, 0);
nf_bridge = nf_bridge_alloc(skb);
if (!nf_bridge)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NOMEM, 0);
if (!setup_pre_routing(skb, state->net))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_DEV_READY, 0);
nf_bridge = nf_bridge_info_get(skb);
nf_bridge->ipv6_daddr = ipv6_hdr(skb)->daddr;
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 7/7] netfilter: nf_tables: de-constify set commit ops function argument
2023-10-18 8:51 [PATCH net-next 0/7] netfilter updates for net-next Florian Westphal
` (5 preceding siblings ...)
2023-10-18 8:51 ` [PATCH net-next 6/7] netfilter: bridge: convert br_netfilter to NF_DROP_REASON Florian Westphal
@ 2023-10-18 8:51 ` Florian Westphal
6 siblings, 0 replies; 9+ messages in thread
From: Florian Westphal @ 2023-10-18 8:51 UTC (permalink / raw)
To: netdev
Cc: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
netfilter-devel
The set backend using this already has to work around this via ugly
cast, don't spread this pattern.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/net/netfilter/nf_tables.h | 2 +-
net/netfilter/nft_set_pipapo.c | 7 +++----
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 9fb16485d08f..8de040d2d2cf 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -462,7 +462,7 @@ struct nft_set_ops {
const struct nft_set *set,
const struct nft_set_elem *elem,
unsigned int flags);
- void (*commit)(const struct nft_set *set);
+ void (*commit)(struct nft_set *set);
void (*abort)(const struct nft_set *set);
u64 (*privsize)(const struct nlattr * const nla[],
const struct nft_set_desc *desc);
diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
index c0dcc40de358..75a9dee353e2 100644
--- a/net/netfilter/nft_set_pipapo.c
+++ b/net/netfilter/nft_set_pipapo.c
@@ -1549,12 +1549,11 @@ static void nft_pipapo_gc_deactivate(struct net *net, struct nft_set *set,
/**
* pipapo_gc() - Drop expired entries from set, destroy start and end elements
- * @_set: nftables API set representation
+ * @set: nftables API set representation
* @m: Matching data
*/
-static void pipapo_gc(const struct nft_set *_set, struct nft_pipapo_match *m)
+static void pipapo_gc(struct nft_set *set, struct nft_pipapo_match *m)
{
- struct nft_set *set = (struct nft_set *) _set;
struct nft_pipapo *priv = nft_set_priv(set);
struct net *net = read_pnet(&set->net);
int rules_f0, first_rule = 0;
@@ -1672,7 +1671,7 @@ static void pipapo_reclaim_match(struct rcu_head *rcu)
* We also need to create a new working copy for subsequent insertions and
* deletions.
*/
-static void nft_pipapo_commit(const struct nft_set *set)
+static void nft_pipapo_commit(struct nft_set *set)
{
struct nft_pipapo *priv = nft_set_priv(set);
struct nft_pipapo_match *new_clone, *old;
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread