* [PATCH net-next 0/2] net: flow dissector: allow explicit passing of netns
@ 2024-06-07 8:31 Florian Westphal
2024-06-07 8:31 ` [PATCH net-next 1/2] net: add and use skb_get_hash_net Florian Westphal
2024-06-07 8:32 ` [PATCH net-next 2/2] net: add and use __skb_get_hash_symmetric_net Florian Westphal
0 siblings, 2 replies; 9+ messages in thread
From: Florian Westphal @ 2024-06-07 8:31 UTC (permalink / raw)
To: netdev
Cc: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
netfilter-devel, pablo, willemb
Years ago flow dissector gained ability to delegate flow dissection
to a bpf program, scoped per netns.
The netns is derived from skb->dev, and if that is not available, from
skb->sk. If neither is set, we hit a (benign) WARN_ON_ONCE().
This WARN_ON_ONCE can be triggered from netfilter.
Known skb origins are nf_send_reset and ipv4 stack generated IGMP
messages.
Lets allow callers to pass the current netns explicitly and make
nf_tables use those instead.
This targets net-next instead of net because the WARN is benign and this
is not a regression.
Florian Westphal (2):
net: add and use skb_get_hash_net
net: add and use __skb_get_hash_symmetric_net
include/linux/skbuff.h | 20 +++++++++++++++++---
net/core/flow_dissector.c | 20 +++++++++++++-------
net/netfilter/nf_tables_trace.c | 2 +-
net/netfilter/nft_hash.c | 3 ++-
4 files changed, 33 insertions(+), 12 deletions(-)
--
2.44.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next 1/2] net: add and use skb_get_hash_net
2024-06-07 8:31 [PATCH net-next 0/2] net: flow dissector: allow explicit passing of netns Florian Westphal
@ 2024-06-07 8:31 ` Florian Westphal
2024-06-07 9:25 ` Eric Dumazet
2024-06-07 12:33 ` kernel test robot
2024-06-07 8:32 ` [PATCH net-next 2/2] net: add and use __skb_get_hash_symmetric_net Florian Westphal
1 sibling, 2 replies; 9+ messages in thread
From: Florian Westphal @ 2024-06-07 8:31 UTC (permalink / raw)
To: netdev
Cc: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
netfilter-devel, pablo, willemb, Christoph Paasch
Years ago flow dissector gained ability to delegate flow dissection
to a bpf program, scoped per netns.
Unfortunately, skb_get_hash() only gets an sk_buff argument instead
of both net+skb. This means the flow dissector needs to obtain the
netns pointer from somewhere else.
The netns is derived from skb->dev, and if that is not available, from
skb->sk. If neither is set, we hit a (benign) WARN_ON_ONCE().
Trying both dev and sk covers most cases, but not all, as recently
reported by Christoph Paasch.
In case of nf-generated tcp reset, both sk and dev are NULL:
WARNING: .. net/core/flow_dissector.c:1104
skb_flow_dissect_flow_keys include/linux/skbuff.h:1536 [inline]
skb_get_hash include/linux/skbuff.h:1578 [inline]
nft_trace_init+0x7d/0x120 net/netfilter/nf_tables_trace.c:320
nft_do_chain+0xb26/0xb90 net/netfilter/nf_tables_core.c:268
nft_do_chain_ipv4+0x7a/0xa0 net/netfilter/nft_chain_filter.c:23
nf_hook_slow+0x57/0x160 net/netfilter/core.c:626
__ip_local_out+0x21d/0x260 net/ipv4/ip_output.c:118
ip_local_out+0x26/0x1e0 net/ipv4/ip_output.c:127
nf_send_reset+0x58c/0x700 net/ipv4/netfilter/nf_reject_ipv4.c:308
nft_reject_ipv4_eval+0x53/0x90 net/ipv4/netfilter/nft_reject_ipv4.c:30
[..]
syzkaller did something like this:
table inet filter {
chain input {
type filter hook input priority filter; policy accept;
meta nftrace set 1 # calls skb_get_hash
tcp dport 42 reject with tcp reset # emits skb with NULL skb dev/sk
}
chain output {
type filter hook output priority filter; policy accept;
# empty chain is enough
}
}
... then sends a tcp packet to port 42.
Initial attempt to simply set skb->dev from nf_reject_ipv4 doesn't cover
all cases: skbs generated via ipv4 igmp_send_report trigger similar splat.
Moreover, Pablo Neira found that nft_hash.c uses __skb_get_hash_symmetric()
which would trigger same warn splat for such skbs.
Lets allow callers to pass the current netns explicitly.
The nf_trace infrastructure is adjusted to use the new helper.
__skb_get_hash_symmetric is handled in the next patch.
Reported-by: Christoph Paasch <cpaasch@apple.com>
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/494
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/linux/skbuff.h | 12 ++++++++++--
net/core/flow_dissector.c | 14 ++++++++++----
net/netfilter/nf_tables_trace.c | 2 +-
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index fe7d8dbef77e..6e78019f899a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1498,7 +1498,7 @@ __skb_set_sw_hash(struct sk_buff *skb, __u32 hash, bool is_l4)
__skb_set_hash(skb, hash, true, is_l4);
}
-void __skb_get_hash(struct sk_buff *skb);
+void __skb_get_hash_net(const struct net *net, struct sk_buff *skb);
u32 __skb_get_hash_symmetric(const struct sk_buff *skb);
u32 skb_get_poff(const struct sk_buff *skb);
u32 __skb_get_poff(const struct sk_buff *skb, const void *data,
@@ -1578,10 +1578,18 @@ void skb_flow_dissect_hash(const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
void *target_container);
+static inline __u32 skb_get_hash_net(const struct net *net, struct sk_buff *skb)
+{
+ if (!skb->l4_hash && !skb->sw_hash)
+ __skb_get_hash_net(net, skb);
+
+ return skb->hash;
+}
+
static inline __u32 skb_get_hash(struct sk_buff *skb)
{
if (!skb->l4_hash && !skb->sw_hash)
- __skb_get_hash(skb);
+ __skb_get_hash_net(NULL, skb);
return skb->hash;
}
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 59fe46077b3c..32454181be60 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -1860,7 +1860,7 @@ u32 __skb_get_hash_symmetric(const struct sk_buff *skb)
EXPORT_SYMBOL_GPL(__skb_get_hash_symmetric);
/**
- * __skb_get_hash: calculate a flow hash
+ * __skb_get_hash_net: calculate a flow hash
* @skb: sk_buff to calculate flow hash from
*
* This function calculates a flow hash based on src/dst addresses
@@ -1868,18 +1868,24 @@ EXPORT_SYMBOL_GPL(__skb_get_hash_symmetric);
* on success, zero indicates no valid hash. Also, sets l4_hash in skb
* if hash is a canonical 4-tuple hash over transport ports.
*/
-void __skb_get_hash(struct sk_buff *skb)
+void __skb_get_hash_net(const struct net *net, struct sk_buff *skb)
{
struct flow_keys keys;
u32 hash;
+ memset(&keys, 0, sizeof(keys));
+
+ __skb_flow_dissect(net, skb, &flow_keys_dissector,
+ &keys, NULL, 0, 0, 0,
+ FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL);
+
__flow_hash_secret_init();
- hash = ___skb_get_hash(skb, &keys, &hashrnd);
+ hash = __flow_hash_from_keys(&keys, &hashrnd);
__skb_set_sw_hash(skb, hash, flow_keys_have_l4(&keys));
}
-EXPORT_SYMBOL(__skb_get_hash);
+EXPORT_SYMBOL(__skb_get_hash_net);
__u32 skb_get_hash_perturb(const struct sk_buff *skb,
const siphash_key_t *perturb)
diff --git a/net/netfilter/nf_tables_trace.c b/net/netfilter/nf_tables_trace.c
index a83637e3f455..580c55268f65 100644
--- a/net/netfilter/nf_tables_trace.c
+++ b/net/netfilter/nf_tables_trace.c
@@ -317,7 +317,7 @@ void nft_trace_init(struct nft_traceinfo *info, const struct nft_pktinfo *pkt,
net_get_random_once(&trace_key, sizeof(trace_key));
info->skbid = (u32)siphash_3u32(hash32_ptr(skb),
- skb_get_hash(skb),
+ skb_get_hash_net(nft_net(pkt), skb),
skb->skb_iif,
&trace_key);
}
--
2.44.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 2/2] net: add and use __skb_get_hash_symmetric_net
2024-06-07 8:31 [PATCH net-next 0/2] net: flow dissector: allow explicit passing of netns Florian Westphal
2024-06-07 8:31 ` [PATCH net-next 1/2] net: add and use skb_get_hash_net Florian Westphal
@ 2024-06-07 8:32 ` Florian Westphal
2024-06-07 9:26 ` Eric Dumazet
1 sibling, 1 reply; 9+ messages in thread
From: Florian Westphal @ 2024-06-07 8:32 UTC (permalink / raw)
To: netdev
Cc: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
netfilter-devel, pablo, willemb
Similar to previous patch: apply same logic for
__skb_get_hash_symmetric and let callers pass the netns to the dissector
core.
Existing function is turned into a wrapper to avoid adjusting all
callers, nft_hash.c uses new function.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/linux/skbuff.h | 8 +++++++-
net/core/flow_dissector.c | 6 +++---
net/netfilter/nft_hash.c | 3 ++-
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6e78019f899a..813406a9bd6c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1498,8 +1498,14 @@ __skb_set_sw_hash(struct sk_buff *skb, __u32 hash, bool is_l4)
__skb_set_hash(skb, hash, true, is_l4);
}
+u32 __skb_get_hash_symmetric_net(const struct net *net, const struct sk_buff *skb);
+
+static inline u32 __skb_get_hash_symmetric(const struct sk_buff *skb)
+{
+ return __skb_get_hash_symmetric_net(NULL, skb);
+}
+
void __skb_get_hash_net(const struct net *net, struct sk_buff *skb);
-u32 __skb_get_hash_symmetric(const struct sk_buff *skb);
u32 skb_get_poff(const struct sk_buff *skb);
u32 __skb_get_poff(const struct sk_buff *skb, const void *data,
const struct flow_keys_basic *keys, int hlen);
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 32454181be60..f6a97aec38da 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -1845,19 +1845,19 @@ EXPORT_SYMBOL(make_flow_keys_digest);
static struct flow_dissector flow_keys_dissector_symmetric __read_mostly;
-u32 __skb_get_hash_symmetric(const struct sk_buff *skb)
+u32 __skb_get_hash_symmetric_net(const struct net *net, const struct sk_buff *skb)
{
struct flow_keys keys;
__flow_hash_secret_init();
memset(&keys, 0, sizeof(keys));
- __skb_flow_dissect(NULL, skb, &flow_keys_dissector_symmetric,
+ __skb_flow_dissect(net, skb, &flow_keys_dissector_symmetric,
&keys, NULL, 0, 0, 0, 0);
return __flow_hash_from_keys(&keys, &hashrnd);
}
-EXPORT_SYMBOL_GPL(__skb_get_hash_symmetric);
+EXPORT_SYMBOL_GPL(__skb_get_hash_symmetric_net);
/**
* __skb_get_hash_net: calculate a flow hash
diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index 92d47e469204..868d68302d22 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -51,7 +51,8 @@ static void nft_symhash_eval(const struct nft_expr *expr,
struct sk_buff *skb = pkt->skb;
u32 h;
- h = reciprocal_scale(__skb_get_hash_symmetric(skb), priv->modulus);
+ h = reciprocal_scale(__skb_get_hash_symmetric_net(nft_net(pkt), skb),
+ priv->modulus);
regs->data[priv->dreg] = h + priv->offset;
}
--
2.44.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/2] net: add and use skb_get_hash_net
2024-06-07 8:31 ` [PATCH net-next 1/2] net: add and use skb_get_hash_net Florian Westphal
@ 2024-06-07 9:25 ` Eric Dumazet
2024-06-07 14:13 ` Willem de Bruijn
2024-06-07 12:33 ` kernel test robot
1 sibling, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2024-06-07 9:25 UTC (permalink / raw)
To: Florian Westphal
Cc: netdev, Paolo Abeni, David S. Miller, Jakub Kicinski,
netfilter-devel, pablo, willemb, Christoph Paasch
On Fri, Jun 7, 2024 at 10:36 AM Florian Westphal <fw@strlen.de> wrote:
>
> Years ago flow dissector gained ability to delegate flow dissection
> to a bpf program, scoped per netns.
>
> Unfortunately, skb_get_hash() only gets an sk_buff argument instead
> of both net+skb. This means the flow dissector needs to obtain the
> netns pointer from somewhere else.
>
> The netns is derived from skb->dev, and if that is not available, from
> skb->sk. If neither is set, we hit a (benign) WARN_ON_ONCE().
>
> Trying both dev and sk covers most cases, but not all, as recently
> reported by Christoph Paasch.
>
> In case of nf-generated tcp reset, both sk and dev are NULL:
>
> WARNING: .. net/core/flow_dissector.c:1104
> skb_flow_dissect_flow_keys include/linux/skbuff.h:1536 [inline]
> skb_get_hash include/linux/skbuff.h:1578 [inline]
> nft_trace_init+0x7d/0x120 net/netfilter/nf_tables_trace.c:320
> nft_do_chain+0xb26/0xb90 net/netfilter/nf_tables_core.c:268
> nft_do_chain_ipv4+0x7a/0xa0 net/netfilter/nft_chain_filter.c:23
> nf_hook_slow+0x57/0x160 net/netfilter/core.c:626
> __ip_local_out+0x21d/0x260 net/ipv4/ip_output.c:118
> ip_local_out+0x26/0x1e0 net/ipv4/ip_output.c:127
> nf_send_reset+0x58c/0x700 net/ipv4/netfilter/nf_reject_ipv4.c:308
> nft_reject_ipv4_eval+0x53/0x90 net/ipv4/netfilter/nft_reject_ipv4.c:30
> [..]
>
> syzkaller did something like this:
> table inet filter {
> chain input {
> type filter hook input priority filter; policy accept;
> meta nftrace set 1 # calls skb_get_hash
> tcp dport 42 reject with tcp reset # emits skb with NULL skb dev/sk
> }
> chain output {
> type filter hook output priority filter; policy accept;
> # empty chain is enough
> }
> }
>
> ... then sends a tcp packet to port 42.
>
> Initial attempt to simply set skb->dev from nf_reject_ipv4 doesn't cover
> all cases: skbs generated via ipv4 igmp_send_report trigger similar splat.
>
> Moreover, Pablo Neira found that nft_hash.c uses __skb_get_hash_symmetric()
> which would trigger same warn splat for such skbs.
>
> Lets allow callers to pass the current netns explicitly.
> The nf_trace infrastructure is adjusted to use the new helper.
>
> __skb_get_hash_symmetric is handled in the next patch.
>
> Reported-by: Christoph Paasch <cpaasch@apple.com>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/494
> Signed-off-by: Florian Westphal <fw@strlen.de>
Nice, I had an internal syzbot report about the same issue.
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 2/2] net: add and use __skb_get_hash_symmetric_net
2024-06-07 8:32 ` [PATCH net-next 2/2] net: add and use __skb_get_hash_symmetric_net Florian Westphal
@ 2024-06-07 9:26 ` Eric Dumazet
2024-06-07 14:14 ` Willem de Bruijn
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2024-06-07 9:26 UTC (permalink / raw)
To: Florian Westphal
Cc: netdev, Paolo Abeni, David S. Miller, Jakub Kicinski,
netfilter-devel, pablo, willemb
On Fri, Jun 7, 2024 at 10:36 AM Florian Westphal <fw@strlen.de> wrote:
>
> Similar to previous patch: apply same logic for
> __skb_get_hash_symmetric and let callers pass the netns to the dissector
> core.
>
> Existing function is turned into a wrapper to avoid adjusting all
> callers, nft_hash.c uses new function.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/2] net: add and use skb_get_hash_net
2024-06-07 8:31 ` [PATCH net-next 1/2] net: add and use skb_get_hash_net Florian Westphal
2024-06-07 9:25 ` Eric Dumazet
@ 2024-06-07 12:33 ` kernel test robot
1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2024-06-07 12:33 UTC (permalink / raw)
To: Florian Westphal, netdev
Cc: oe-kbuild-all, Paolo Abeni, Eric Dumazet, Jakub Kicinski,
netfilter-devel, pablo, willemb, Christoph Paasch
Hi Florian,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Florian-Westphal/net-add-and-use-skb_get_hash_net/20240607-163738
base: net-next/main
patch link: https://lore.kernel.org/r/20240607083205.3000-2-fw%40strlen.de
patch subject: [PATCH net-next 1/2] net: add and use skb_get_hash_net
config: openrisc-defconfig (https://download.01.org/0day-ci/archive/20240607/202406072022.OkRGOAuS-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/202406072022.OkRGOAuS-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406072022.OkRGOAuS-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> net/core/flow_dissector.c:1872: warning: Function parameter or struct member 'net' not described in '__skb_get_hash_net'
vim +1872 net/core/flow_dissector.c
eb70db8756717b David S. Miller 2016-07-01 1861
d4fd32757176d1 Jiri Pirko 2015-05-12 1862 /**
11b45a5b56dab6 Florian Westphal 2024-06-07 1863 * __skb_get_hash_net: calculate a flow hash
d4fd32757176d1 Jiri Pirko 2015-05-12 1864 * @skb: sk_buff to calculate flow hash from
d4fd32757176d1 Jiri Pirko 2015-05-12 1865 *
d4fd32757176d1 Jiri Pirko 2015-05-12 1866 * This function calculates a flow hash based on src/dst addresses
61b905da33ae25 Tom Herbert 2014-03-24 1867 * and src/dst port numbers. Sets hash in skb to non-zero hash value
61b905da33ae25 Tom Herbert 2014-03-24 1868 * on success, zero indicates no valid hash. Also, sets l4_hash in skb
441d9d327f1e77 Cong Wang 2013-01-21 1869 * if hash is a canonical 4-tuple hash over transport ports.
441d9d327f1e77 Cong Wang 2013-01-21 1870 */
11b45a5b56dab6 Florian Westphal 2024-06-07 1871 void __skb_get_hash_net(const struct net *net, struct sk_buff *skb)
441d9d327f1e77 Cong Wang 2013-01-21 @1872 {
441d9d327f1e77 Cong Wang 2013-01-21 1873 struct flow_keys keys;
635c223cfa05af Gao Feng 2016-08-31 1874 u32 hash;
441d9d327f1e77 Cong Wang 2013-01-21 1875
11b45a5b56dab6 Florian Westphal 2024-06-07 1876 memset(&keys, 0, sizeof(keys));
11b45a5b56dab6 Florian Westphal 2024-06-07 1877
11b45a5b56dab6 Florian Westphal 2024-06-07 1878 __skb_flow_dissect(net, skb, &flow_keys_dissector,
11b45a5b56dab6 Florian Westphal 2024-06-07 1879 &keys, NULL, 0, 0, 0,
11b45a5b56dab6 Florian Westphal 2024-06-07 1880 FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL);
11b45a5b56dab6 Florian Westphal 2024-06-07 1881
50fb799289501c Tom Herbert 2015-05-01 1882 __flow_hash_secret_init();
50fb799289501c Tom Herbert 2015-05-01 1883
11b45a5b56dab6 Florian Westphal 2024-06-07 1884 hash = __flow_hash_from_keys(&keys, &hashrnd);
635c223cfa05af Gao Feng 2016-08-31 1885
635c223cfa05af Gao Feng 2016-08-31 1886 __skb_set_sw_hash(skb, hash, flow_keys_have_l4(&keys));
441d9d327f1e77 Cong Wang 2013-01-21 1887 }
11b45a5b56dab6 Florian Westphal 2024-06-07 1888 EXPORT_SYMBOL(__skb_get_hash_net);
441d9d327f1e77 Cong Wang 2013-01-21 1889
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/2] net: add and use skb_get_hash_net
2024-06-07 9:25 ` Eric Dumazet
@ 2024-06-07 14:13 ` Willem de Bruijn
2024-06-08 22:17 ` Florian Westphal
0 siblings, 1 reply; 9+ messages in thread
From: Willem de Bruijn @ 2024-06-07 14:13 UTC (permalink / raw)
To: Eric Dumazet, Florian Westphal
Cc: netdev, Paolo Abeni, David S. Miller, Jakub Kicinski,
netfilter-devel, pablo, willemb, Christoph Paasch
Eric Dumazet wrote:
> On Fri, Jun 7, 2024 at 10:36 AM Florian Westphal <fw@strlen.de> wrote:
> >
> > Years ago flow dissector gained ability to delegate flow dissection
> > to a bpf program, scoped per netns.
> >
> > Unfortunately, skb_get_hash() only gets an sk_buff argument instead
> > of both net+skb. This means the flow dissector needs to obtain the
> > netns pointer from somewhere else.
> >
> > The netns is derived from skb->dev, and if that is not available, from
> > skb->sk. If neither is set, we hit a (benign) WARN_ON_ONCE().
> >
> > Trying both dev and sk covers most cases, but not all, as recently
> > reported by Christoph Paasch.
> >
> > In case of nf-generated tcp reset, both sk and dev are NULL:
> >
> > WARNING: .. net/core/flow_dissector.c:1104
> > skb_flow_dissect_flow_keys include/linux/skbuff.h:1536 [inline]
> > skb_get_hash include/linux/skbuff.h:1578 [inline]
> > nft_trace_init+0x7d/0x120 net/netfilter/nf_tables_trace.c:320
> > nft_do_chain+0xb26/0xb90 net/netfilter/nf_tables_core.c:268
> > nft_do_chain_ipv4+0x7a/0xa0 net/netfilter/nft_chain_filter.c:23
> > nf_hook_slow+0x57/0x160 net/netfilter/core.c:626
> > __ip_local_out+0x21d/0x260 net/ipv4/ip_output.c:118
> > ip_local_out+0x26/0x1e0 net/ipv4/ip_output.c:127
> > nf_send_reset+0x58c/0x700 net/ipv4/netfilter/nf_reject_ipv4.c:308
> > nft_reject_ipv4_eval+0x53/0x90 net/ipv4/netfilter/nft_reject_ipv4.c:30
> > [..]
> >
> > syzkaller did something like this:
> > table inet filter {
> > chain input {
> > type filter hook input priority filter; policy accept;
> > meta nftrace set 1 # calls skb_get_hash
> > tcp dport 42 reject with tcp reset # emits skb with NULL skb dev/sk
> > }
> > chain output {
> > type filter hook output priority filter; policy accept;
> > # empty chain is enough
> > }
> > }
> >
> > ... then sends a tcp packet to port 42.
> >
> > Initial attempt to simply set skb->dev from nf_reject_ipv4 doesn't cover
> > all cases: skbs generated via ipv4 igmp_send_report trigger similar splat.
Does this mean we have more non-nf callsites to convert?
> >
> > Moreover, Pablo Neira found that nft_hash.c uses __skb_get_hash_symmetric()
> > which would trigger same warn splat for such skbs.
> >
> > Lets allow callers to pass the current netns explicitly.
> > The nf_trace infrastructure is adjusted to use the new helper.
> >
> > __skb_get_hash_symmetric is handled in the next patch.
> >
> > Reported-by: Christoph Paasch <cpaasch@apple.com>
> > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/494
> > Signed-off-by: Florian Westphal <fw@strlen.de>
>
> Nice, I had an internal syzbot report about the same issue.
>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
Subject to the documentation warning from the bot
Reviewed-by: Willem de Bruijn <willemb@google.com>
Thanks for fixing this, Florian.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 2/2] net: add and use __skb_get_hash_symmetric_net
2024-06-07 9:26 ` Eric Dumazet
@ 2024-06-07 14:14 ` Willem de Bruijn
0 siblings, 0 replies; 9+ messages in thread
From: Willem de Bruijn @ 2024-06-07 14:14 UTC (permalink / raw)
To: Eric Dumazet, Florian Westphal
Cc: netdev, Paolo Abeni, David S. Miller, Jakub Kicinski,
netfilter-devel, pablo, willemb
Eric Dumazet wrote:
> On Fri, Jun 7, 2024 at 10:36 AM Florian Westphal <fw@strlen.de> wrote:
> >
> > Similar to previous patch: apply same logic for
> > __skb_get_hash_symmetric and let callers pass the netns to the dissector
> > core.
> >
> > Existing function is turned into a wrapper to avoid adjusting all
> > callers, nft_hash.c uses new function.
> >
> > Signed-off-by: Florian Westphal <fw@strlen.de>
>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/2] net: add and use skb_get_hash_net
2024-06-07 14:13 ` Willem de Bruijn
@ 2024-06-08 22:17 ` Florian Westphal
0 siblings, 0 replies; 9+ messages in thread
From: Florian Westphal @ 2024-06-08 22:17 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Eric Dumazet, Florian Westphal, netdev, Paolo Abeni,
David S. Miller, Jakub Kicinski, netfilter-devel, pablo, willemb,
Christoph Paasch
Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> > > syzkaller did something like this:
> > > table inet filter {
> > > chain input {
> > > type filter hook input priority filter; policy accept;
> > > meta nftrace set 1 # calls skb_get_hash
> > > tcp dport 42 reject with tcp reset # emits skb with NULL skb dev/sk
> > > }
> > > chain output {
> > > type filter hook output priority filter; policy accept;
> > > # empty chain is enough
> > > }
> > > }
> > >
> > > ... then sends a tcp packet to port 42.
> > >
> > > Initial attempt to simply set skb->dev from nf_reject_ipv4 doesn't cover
> > > all cases: skbs generated via ipv4 igmp_send_report trigger similar splat.
>
> Does this mean we have more non-nf callsites to convert?
There might be non-nf call sites that need skb_get_hash_net(),
but I don't know of any.
The above comment was meant to say that I tried to patch this
outside of flow dissector by setting skb->dev properly in nf_reject,
but that still triggers a slightly different WARN trace, this time
due to igmp_send_report also sending skb without dev+sk pointers.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-06-08 22:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-07 8:31 [PATCH net-next 0/2] net: flow dissector: allow explicit passing of netns Florian Westphal
2024-06-07 8:31 ` [PATCH net-next 1/2] net: add and use skb_get_hash_net Florian Westphal
2024-06-07 9:25 ` Eric Dumazet
2024-06-07 14:13 ` Willem de Bruijn
2024-06-08 22:17 ` Florian Westphal
2024-06-07 12:33 ` kernel test robot
2024-06-07 8:32 ` [PATCH net-next 2/2] net: add and use __skb_get_hash_symmetric_net Florian Westphal
2024-06-07 9:26 ` Eric Dumazet
2024-06-07 14:14 ` Willem de Bruijn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).