From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2021-02-05 00:18:27
Hi,
The following patchset contains Netfilter fixes for net:
1) Fix combination of --reap and --update in xt_recent that triggers
UAF, from Jozsef Kadlecsik.
2) Fix current year in nft_meta selftest, from Fabian Frederick.
3) Fix possible UAF in the netns destroy path of nftables.
4) Fix incorrect checksum calculation when mangling ports in flowtable,
from Sven Auhagen.
Please, pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
Thanks!
----------------------------------------------------------------
The following changes since commit 44a674d6f79867d5652026f1cc11f7ba8a390183:
Merge tag 'mlx5-fixes-2021-01-26' of git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux (2021-01-27 19:18:37 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git HEAD
for you to fetch changes up to 8d6bca156e47d68551750a384b3ff49384c67be3:
netfilter: flowtable: fix tcp and udp header checksum update (2021-02-04 01:10:14 +0100)
----------------------------------------------------------------
Fabian Frederick (1):
selftests: netfilter: fix current year
Jozsef Kadlecsik (1):
netfilter: xt_recent: Fix attempt to update deleted entry
Pablo Neira Ayuso (1):
netfilter: nftables: fix possible UAF over chains from packet path in netns
Sven Auhagen (1):
netfilter: flowtable: fix tcp and udp header checksum update
net/netfilter/nf_flow_table_core.c | 4 ++--
net/netfilter/nf_tables_api.c | 25 +++++++++++++++++++------
net/netfilter/xt_recent.c | 12 ++++++++++--
tools/testing/selftests/netfilter/nft_meta.sh | 2 +-
4 files changed, 32 insertions(+), 11 deletions(-)
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2021-02-05 00:18:21
Although hooks are released via call_rcu(), chain and rule objects are
immediately released while packets are still walking over these bits.
This patch adds the .pre_exit callback which is invoked before
synchronize_rcu() in the netns framework to stay safe.
Remove a comment which is not valid anymore since the core does not use
synchronize_net() anymore since 8c873e219970 ("netfilter: core: free
hooks with call_rcu").
Suggested-by: Florian Westphal <fw@strlen.de>
Fixes: df05ef874b28 ("netfilter: nf_tables: release objects on netns destruction")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_tables_api.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
@@ -8949,6 +8949,17 @@ int __nft_release_basechain(struct nft_ctx *ctx)}EXPORT_SYMBOL_GPL(__nft_release_basechain);+staticvoid__nft_release_hooks(structnet*net)+{+structnft_table*table;+structnft_chain*chain;++list_for_each_entry(table,&net->nft.tables,list){+list_for_each_entry(chain,&table->chains,list)+nf_tables_unregister_hook(net,table,chain);+}+}+staticvoid__nft_release_tables(structnet*net){structnft_flowtable*flowtable,*nf;
@@ -8964,10 +8975,6 @@ static void __nft_release_tables(struct net *net)list_for_each_entry_safe(table,nt,&net->nft.tables,list){ctx.family=table->family;--list_for_each_entry(chain,&table->chains,list)-nf_tables_unregister_hook(net,table,chain);-/* No packets are walking on these chains anymore. */ctx.table=table;list_for_each_entry(chain,&table->chains,list){ctx.chain=chain;
@@ -9016,6 +9023,11 @@ static int __net_init nf_tables_init_net(struct net *net)return0;}+staticvoid__net_exitnf_tables_pre_exit_net(structnet*net)+{+__nft_release_hooks(net);+}+staticvoid__net_exitnf_tables_exit_net(structnet*net){mutex_lock(&net->nft.commit_mutex);
@@ -9029,8 +9041,9 @@ static void __net_exit nf_tables_exit_net(struct net *net)}staticstructpernet_operationsnf_tables_net_ops={-.init=nf_tables_init_net,-.exit=nf_tables_exit_net,+.init=nf_tables_init_net,+.pre_exit=nf_tables_pre_exit_net,+.exit=nf_tables_exit_net,};staticint__initnf_tables_module_init(void)
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2021-02-05 00:18:24
From: Sven Auhagen <redacted>
When updating the tcp or udp header checksum on port nat the function
inet_proto_csum_replace2 with the last parameter pseudohdr as true.
This leads to an error in the case that GRO is used and packets are
split up in GSO. The tcp or udp checksum of all packets is incorrect.
The error is probably masked due to the fact the most network driver
implement tcp/udp checksum offloading. It also only happens when GRO is
applied and not on single packets.
The error is most visible when using a pppoe connection which is not
triggering the tcp/udp checksum offload.
Fixes: ac2a66665e23 ("netfilter: add generic flow table infrastructure")
Signed-off-by: Sven Auhagen <redacted>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_flow_table_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2021-02-05 00:18:34
From: Fabian Frederick <redacted>
use date %Y instead of %G to read current year
Problem appeared when running lkp-tests on 01/01/2021
Fixes: 48d072c4e8cd ("selftests: netfilter: add time counter check")
Reported-by: kernel test robot <redacted>
Signed-off-by: Fabian Frederick <redacted>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
tools/testing/selftests/netfilter/nft_meta.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2021-02-05 00:18:35
From: Jozsef Kadlecsik <redacted>
When both --reap and --update flag are specified, there's a code
path at which the entry to be updated is reaped beforehand,
which then leads to kernel crash. Reap only entries which won't be
updated.
Fixes kernel bugzilla #207773.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=207773
Reported-by: Reindl Harald <redacted>
Fixes: 0079c5aee348 ("netfilter: xt_recent: add an entry reaper")
Signed-off-by: Jozsef Kadlecsik <redacted>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/xt_recent.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
Hello:
This series was applied to netdev/net.git (refs/heads/master):
On Fri, 5 Feb 2021 01:17:24 +0100 you wrote:
From: Jozsef Kadlecsik <redacted>
When both --reap and --update flag are specified, there's a code
path at which the entry to be updated is reaped beforehand,
which then leads to kernel crash. Reap only entries which won't be
updated.
[...]
From: Reindl Harald <hidden> Date: 2021-02-05 11:58:17
thank you for adressing that issue - maybe GRO can be enabled and wasn't
involved at all
"Reap only entries which won't be updated" sounds for me like the could
be some optimization: i mean when you first update and then check what
can be reaped the recently updated entry would not match to begin with
Am 05.02.21 um 01:17 schrieb Pablo Neira Ayuso:
quoted hunk
From: Jozsef Kadlecsik <redacted>
When both --reap and --update flag are specified, there's a code
path at which the entry to be updated is reaped beforehand,
which then leads to kernel crash. Reap only entries which won't be
updated.
Fixes kernel bugzilla #207773.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=207773
Reported-by: Reindl Harald <redacted>
Fixes: 0079c5aee348 ("netfilter: xt_recent: add an entry reaper")
Signed-off-by: Jozsef Kadlecsik <redacted>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/xt_recent.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
From: Reindl Harald <hidden> Date: 2021-02-05 18:46:09
Am 05.02.21 um 14:54 schrieb Jozsef Kadlecsik:
Hi Harald,
On Fri, 5 Feb 2021, Reindl Harald wrote:
quoted
"Reap only entries which won't be updated" sounds for me like the could
be some optimization: i mean when you first update and then check what
can be reaped the recently updated entry would not match to begin with
When the entry is new and the given recent table is full we cannot update
(add) it, unless old entries are deleted (reaped) first. So it'd require
more additional checkings to be introduced to reverse the order of the two
operations.
well, the most important thing is that the firewall-vm stops to
kernel-panic, built that beast in autumn 2018 and until april 2019 i
went trough hell with random crashes all the time (connlimit regression,
driver issues, vmware issues and that one where i removed --reap on the
most called one with some other changes when it crashed 5 or 10 times a
day and then 3 days not at all so never figured out what was the gamechanger
on the other hand if you can't reap old entries because everything is
fresh (real DDOS) you can't update / add it anyways
what makes me thinking about the ones without --reap - how is it
handeled in that case, i mean there must be some LRU logic present
anyways given that --reap is not enabled by default (otherwise that bug
would not have hitted me so long randomly)
my first xt_recent-rule on top don't have --reap by intention because
it's the DDOS stuff with total connections to any machine per two
seconds, my guess what that --reap don't come for free and the
roudnabout 200 MB RAM overhead is OK, for the other 12 not hitting that
much the VM would consume 1.5 GB RAM after a few days instead 240 MB -
but they where obviosuly the trigger for random crashes
how does that one work after "it's full" to track recent attackers
instead just consume memory and no longer work properly?
Hi Harald,
On Fri, 5 Feb 2021, Reindl Harald wrote:
"Reap only entries which won't be updated" sounds for me like the could
be some optimization: i mean when you first update and then check what
can be reaped the recently updated entry would not match to begin with
When the entry is new and the given recent table is full we cannot update
(add) it, unless old entries are deleted (reaped) first. So it'd require
more additional checkings to be introduced to reverse the order of the two
operations.
Best regards,
Jozsef
Am 05.02.21 um 01:17 schrieb Pablo Neira Ayuso:
quoted
From: Jozsef Kadlecsik <redacted>
When both --reap and --update flag are specified, there's a code
path at which the entry to be updated is reaped beforehand,
which then leads to kernel crash. Reap only entries which won't be
updated.
Fixes kernel bugzilla #207773.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=207773
Reported-by: Reindl Harald <redacted>
Fixes: 0079c5aee348 ("netfilter: xt_recent: add an entry reaper")
Signed-off-by: Jozsef Kadlecsik <redacted>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/xt_recent.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
struct recent_entry *e)
/*
* Drop entries with timestamps older then 'time'.
*/
-static void recent_entry_reap(struct recent_table *t, unsigned long time)
+static void recent_entry_reap(struct recent_table *t, unsigned long time,
+ struct recent_entry *working, bool update)
{
struct recent_entry *e;
@@ -161,6 +162,12 @@ static void recent_entry_reap(struct recent_table *t,
unsigned long time)
*/
e = list_entry(t->lru_list.next, struct recent_entry, lru_list);
+ /*
+ * Do not reap the entry which are going to be updated.
+ */
+ if (e == working && update)
+ return;
+
/*
* The last time stamp is the most recent.
*/
From: Reindl Harald <hidden> Date: 2021-02-07 16:35:56
Am 05.02.21 um 15:42 schrieb Reindl Harald:
Am 05.02.21 um 14:54 schrieb Jozsef Kadlecsik:
quoted
Hi Harald,
On Fri, 5 Feb 2021, Reindl Harald wrote:
quoted
"Reap only entries which won't be updated" sounds for me like the could
be some optimization: i mean when you first update and then check what
can be reaped the recently updated entry would not match to begin with
When the entry is new and the given recent table is full we cannot update
(add) it, unless old entries are deleted (reaped) first. So it'd require
more additional checkings to be introduced to reverse the order of the
two
operations.
well, the most important thing is that the firewall-vm stops to
kernel-panic
what makes me thinking about the ones without --reap - how is it
handeled in that case, i mean there must be some LRU logic present
anyways given that --reap is not enabled by default (otherwise that bug
would not have hitted me so long randomly)
Yes, checking the code I was wrong: when the recent table is full, the
oldest entry is automatically removed to make space for the new one.
Best regards,
Jozsef
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.hu
PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics
H-1525 Budapest 114, POB. 49, Hungary
Probably we missed the window when patches were accepted for the new
release. That's all.
Best regards,
Jozsef
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.hu
PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics
H-1525 Budapest 114, POB. 49, Hungary
Probably we missed the window when patches were accepted for the new
release. That's all
probably something is broken in the whole process given that 5.10.15
still don't contain the fix while i am tired of a new "stable release"
every few days and 5.10.x like every LTS release in the past few years
has a peak of it
https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.15
Probably we missed the window when patches were accepted for the new
release. That's all
probably something is broken in the whole process given that 5.10.15
still don't contain the fix while i am tired of a new "stable release"
every few days and 5.10.x like every LTS release in the past few years
has a peak of it
https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.15
Probably we missed the window when patches were accepted for the new
release. That's all
probably something is broken in the whole process given that 5.10.15
still don't contain the fix while i am tired of a new "stable release"
every few days and 5.10.x like every LTS release in the past few years
has a peak of it
https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.15
Probably we missed the window when patches were accepted for the new
release. That's all
probably something is broken in the whole process given that 5.10.15 still
don't contain the fix while i am tired of a new "stable release" every few
days and 5.10.x like every LTS release in the past few years has a peak of it
https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.15
The process is a multi-step one: netfilter patches are sent for
reviewing/accepting/rejecting to the net maintaners, and after that they
send the patches to the kernel source maintainers. A single patch is
rarely picked out to handle differently.
The patch has entered the queue for the stable trees.
Best regards,
Jozsef
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.hu
PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics
H-1525 Budapest 114, POB. 49, Hungary