[PATCH net 0/4] Netfilter fixes for net

[PATCH net 0/4] Netfilter fixes for net

[Pablo Neira Ayuso]

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(-)

[PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[Pablo Neira Ayuso]

From: Jozsef Kadlecsik <kadlec@mail.kfki.hu>

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 <h.reindl@thelounge.net>
Fixes: 0079c5aee348 ("netfilter: xt_recent: add an entry reaper")
Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/xt_recent.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
index 606411869698..0446307516cd 100644
--- a/net/netfilter/xt_recent.c
+++ b/net/netfilter/xt_recent.c
@@ -152,7 +152,8 @@ static void recent_entry_remove(struct recent_table *t, 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.
 	 */
@@ -303,7 +310,8 @@ recent_mt(const struct sk_buff *skb, struct xt_action_param *par)
 
 		/* info->seconds must be non-zero */
 		if (info->check_set & XT_RECENT_REAP)
-			recent_entry_reap(t, time);
+			recent_entry_reap(t, time, e,
+				info->check_set & XT_RECENT_UPDATE && ret);
 	}
 
 	if (info->check_set & XT_RECENT_SET ||
-- 
2.20.1

Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[patchwork-bot+netdevbpf]

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 <kadlec@mail.kfki.hu>
> 
> 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.
> 
> [...]

Here is the summary with links:
  - [net,1/4] netfilter: xt_recent: Fix attempt to update deleted entry
    https://git.kernel.org/netdev/net/c/b1bdde33b723
  - [net,2/4] selftests: netfilter: fix current year
    https://git.kernel.org/netdev/net/c/a3005b0f83f2
  - [net,3/4] netfilter: nftables: fix possible UAF over chains from packet path in netns
    https://git.kernel.org/netdev/net/c/767d1216bff8
  - [net,4/4] netfilter: flowtable: fix tcp and udp header checksum update
    https://git.kernel.org/netdev/net/c/8d6bca156e47

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

Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[Reindl Harald]

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:
> From: Jozsef Kadlecsik <kadlec@mail.kfki.hu>
> 
> 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 <h.reindl@thelounge.net>
> Fixes: 0079c5aee348 ("netfilter: xt_recent: add an entry reaper")
> Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>   net/netfilter/xt_recent.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
> index 606411869698..0446307516cd 100644
> --- a/net/netfilter/xt_recent.c
> +++ b/net/netfilter/xt_recent.c
> @@ -152,7 +152,8 @@ static void recent_entry_remove(struct recent_table *t, 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.
>   	 */
> @@ -303,7 +310,8 @@ recent_mt(const struct sk_buff *skb, struct xt_action_param *par)
>   
>   		/* info->seconds must be non-zero */
>   		if (info->check_set & XT_RECENT_REAP)
> -			recent_entry_reap(t, time);
> +			recent_entry_reap(t, time, e,
> +				info->check_set & XT_RECENT_UPDATE && ret);
>   	}
>   
>   	if (info->check_set & XT_RECENT_SET ||

Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[Jozsef Kadlecsik]

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:
> > From: Jozsef Kadlecsik <kadlec@mail.kfki.hu>
> > 
> > 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 <h.reindl@thelounge.net>
> > Fixes: 0079c5aee348 ("netfilter: xt_recent: add an entry reaper")
> > Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> >   net/netfilter/xt_recent.c | 12 ++++++++++--
> >   1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
> > index 606411869698..0446307516cd 100644
> > --- a/net/netfilter/xt_recent.c
> > +++ b/net/netfilter/xt_recent.c
> > @@ -152,7 +152,8 @@ static void recent_entry_remove(struct recent_table *t,
> > 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.
> >   	 */
> > @@ -303,7 +310,8 @@ recent_mt(const struct sk_buff *skb, struct
> > xt_action_param *par)
> >     		/* info->seconds must be non-zero */
> >   		if (info->check_set & XT_RECENT_REAP)
> > -			recent_entry_reap(t, time);
> > +			recent_entry_reap(t, time, e,
> > +				info->check_set & XT_RECENT_UPDATE && ret);
> >   	}
> >     	if (info->check_set & XT_RECENT_SET ||
> 

-
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

Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[Reindl Harald]

Am 05.02.21 um 14:54 schrieb Jozsef Kadlecsik:
> 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.
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?

Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[Reindl Harald]

Am 05.02.21 um 15:42 schrieb Reindl Harald:
> 
> 
> Am 05.02.21 um 14:54 schrieb Jozsef Kadlecsik:
>> 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.
> well, the most important thing is that the firewall-vm stops to 
> kernel-panic

why is that still not part of 5.10.14 given how old that issue is :-(

https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.14

Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[Jozsef Kadlecsik]

On Sun, 7 Feb 2021, Reindl Harald wrote:

> > well, the most important thing is that the firewall-vm stops to 
> > kernel-panic
> 
> why is that still not part of 5.10.14 given how old that issue is :-(
> 
> https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.14

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

Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[Reindl Harald]

Am 07.02.21 um 20:38 schrieb Jozsef Kadlecsik:
> On Sun, 7 Feb 2021, Reindl Harald wrote:
> 
>>> well, the most important thing is that the firewall-vm stops to
>>> kernel-panic
>>
>> why is that still not part of 5.10.14 given how old that issue is :-(
>>
>> https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.14
> 
> 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

Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[Reindl Harald]

Am 10.02.21 um 11:34 schrieb Reindl Harald:
> 
> 
> Am 07.02.21 um 20:38 schrieb Jozsef Kadlecsik:
>> On Sun, 7 Feb 2021, Reindl Harald wrote:
>>
>>>> well, the most important thing is that the firewall-vm stops to
>>>> kernel-panic
>>>
>>> why is that still not part of 5.10.14 given how old that issue is :-(
>>>
>>> https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.14
>>
>> 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

and another useless crash of something which has a ready patch from 
before 5.10.14

[165940.842226] kernel BUG at lib/list_debug.c:45!
[165940.874769] invalid opcode: 0000 [#1] SMP NOPTI
[165940.876680] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
5.10.15-100.fc32.x86_64 #1
[165940.880198] Hardware name: VMware, Inc. VMware Virtual 
Platform/440BX Desktop Reference Platform, BIOS 6.00 12/12/2018
[165940.885314] RIP: 0010:__list_del_entry_valid.cold+0xf/0x47
[165940.886202] Code: fe ff 0f 0b 48 89 d1 4c 89 c6 4c 89 ca 48 c7 c7 60 
88 40 b2 e8 cf 45 fe ff 0f 0b 48 89 fe 48 c7 c7 f0 88 40 b2 e8 be 45 fe 
ff <0f> 0b 48 c7 c7 a0 89 40 b2 e8 b0 45 fe ff 0f 0b 48 89 f2 48 89 fe
[165940.889107] RSP: 0018:ffffaf0480003928 EFLAGS: 00010282
[165940.889943] RAX: 000000000000004e RBX: ffff9fa911148000 RCX: 
0000000000000000
[165940.891066] RDX: ffff9fa99d4269e0 RSI: ffff9fa99d418a80 RDI: 
0000000000000300
[165940.892190] RBP: ffffaf04800039a0 R08: 0000000000000000 R09: 
ffffaf0480003760
[165940.893313] R10: ffffaf0480003758 R11: ffffffffb2b44748 R12: 
ffff9fa9046000f8
[165940.894441] R13: ffff9fa911148010 R14: ffff9fa903329400 R15: 
ffff9fa904600000
[165940.895573] FS:  0000000000000000(0000) GS:ffff9fa99d400000(0000) 
knlGS:0000000000000000
[165940.896856] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[165940.897789] CR2: 00007fb9442e5000 CR3: 00000000030a0006 CR4: 
00000000003706f0
[165940.898954] Call Trace:
[165940.899400]  <IRQ>
[165940.899757]  recent_mt+0x1b5/0x39b [xt_recent]
[165940.900492]  ? set_match_v4+0x92/0xb0 [xt_set]
[165940.901236]  nft_match_large_eval+0x34/0x60 [nft_compat]
[165940.902104]  nft_do_chain+0x141/0x4e0 [nf_tables]
[165940.902869]  ? fib_validate_source+0x47/0xf0
[165940.903564]  ? ip_route_input_slow+0x722/0xaa0
[165940.904282]  nft_do_chain_ipv4+0x56/0x60 [nf_tables]
[165940.905086]  nf_hook_slow+0x3f/0xb0
[165940.905658]  ip_forward+0x441/0x480
[165940.906230]  ? ip4_key_hashfn+0xb0/0xb0
[165940.906856]  __netif_receive_skb_one_core+0x67/0x70
[165940.907639]  netif_receive_skb+0x35/0x110
[165940.908295]  br_handle_frame_finish+0x17a/0x450 [bridge]
[165940.909143]  ? ip_finish_output2+0x19b/0x560
[165940.909842]  ? br_handle_frame_finish+0x450/0x450 [bridge]
[165940.910718]  br_handle_frame+0x292/0x350 [bridge]
[165940.911483]  ? ip_sublist_rcv_finish+0x57/0x70
[165940.912199]  ? ___slab_alloc+0x127/0x5b0
[165940.912835]  __netif_receive_skb_core+0x196/0xf70
[165940.913590]  ? ip_list_rcv+0x125/0x140
[165940.914201]  __netif_receive_skb_list_core+0x12f/0x2b0
[165940.915024]  netif_receive_skb_list_internal+0x1bc/0x2e0
[165940.915873]  ? vmxnet3_rq_rx_complete+0x8bd/0xde0 [vmxnet3]
[165940.916769]  napi_complete_done+0x6f/0x190
[165940.917439]  vmxnet3_poll_rx_only+0x7b/0xa0 [vmxnet3]
[165940.918249]  net_rx_action+0x135/0x3b0
[165940.918863]  __do_softirq+0xca/0x288
[165940.919451]  asm_call_irq_on_stack+0xf/0x20
[165940.920146]  </IRQ>
[165940.920508]  do_softirq_own_stack+0x37/0x40
[165940.921187]  irq_exit_rcu+0xc2/0x100
[165940.921772]  common_interrupt+0x74/0x130
[165940.922410]  asm_common_interrupt+0x1e/0x40

Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[Reindl Harald]

Am 13.02.21 um 17:09 schrieb Reindl Harald:
> 
> 
> Am 10.02.21 um 11:34 schrieb Reindl Harald:
>>
>>
>> Am 07.02.21 um 20:38 schrieb Jozsef Kadlecsik:
>>> On Sun, 7 Feb 2021, Reindl Harald wrote:
>>>
>>>>> well, the most important thing is that the firewall-vm stops to
>>>>> kernel-panic
>>>>
>>>> why is that still not part of 5.10.14 given how old that issue is :-(
>>>>
>>>> https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.14
>>>
>>> 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

https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.16

again no "netfilter" or "xt_recent"

what is the point of new kernel releases every second day without fixing 
months old issues where a pacth exists?

> and another useless crash of something which has a ready patch from 
> before 5.10.14
> 
> [165940.842226] kernel BUG at lib/list_debug.c:45!
> [165940.874769] invalid opcode: 0000 [#1] SMP NOPTI
> [165940.876680] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
> 5.10.15-100.fc32.x86_64 #1
> [165940.880198] Hardware name: VMware, Inc. VMware Virtual 
> Platform/440BX Desktop Reference Platform, BIOS 6.00 12/12/2018
> [165940.885314] RIP: 0010:__list_del_entry_valid.cold+0xf/0x47
> [165940.886202] Code: fe ff 0f 0b 48 89 d1 4c 89 c6 4c 89 ca 48 c7 c7 60 
> 88 40 b2 e8 cf 45 fe ff 0f 0b 48 89 fe 48 c7 c7 f0 88 40 b2 e8 be 45 fe 
> ff <0f> 0b 48 c7 c7 a0 89 40 b2 e8 b0 45 fe ff 0f 0b 48 89 f2 48 89 fe
> [165940.889107] RSP: 0018:ffffaf0480003928 EFLAGS: 00010282
> [165940.889943] RAX: 000000000000004e RBX: ffff9fa911148000 RCX: 
> 0000000000000000
> [165940.891066] RDX: ffff9fa99d4269e0 RSI: ffff9fa99d418a80 RDI: 
> 0000000000000300
> [165940.892190] RBP: ffffaf04800039a0 R08: 0000000000000000 R09: 
> ffffaf0480003760
> [165940.893313] R10: ffffaf0480003758 R11: ffffffffb2b44748 R12: 
> ffff9fa9046000f8
> [165940.894441] R13: ffff9fa911148010 R14: ffff9fa903329400 R15: 
> ffff9fa904600000
> [165940.895573] FS:  0000000000000000(0000) GS:ffff9fa99d400000(0000) 
> knlGS:0000000000000000
> [165940.896856] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [165940.897789] CR2: 00007fb9442e5000 CR3: 00000000030a0006 CR4: 
> 00000000003706f0
> [165940.898954] Call Trace:
> [165940.899400]  <IRQ>
> [165940.899757]  recent_mt+0x1b5/0x39b [xt_recent]
> [165940.900492]  ? set_match_v4+0x92/0xb0 [xt_set]
> [165940.901236]  nft_match_large_eval+0x34/0x60 [nft_compat]
> [165940.902104]  nft_do_chain+0x141/0x4e0 [nf_tables]
> [165940.902869]  ? fib_validate_source+0x47/0xf0
> [165940.903564]  ? ip_route_input_slow+0x722/0xaa0
> [165940.904282]  nft_do_chain_ipv4+0x56/0x60 [nf_tables]
> [165940.905086]  nf_hook_slow+0x3f/0xb0
> [165940.905658]  ip_forward+0x441/0x480
> [165940.906230]  ? ip4_key_hashfn+0xb0/0xb0
> [165940.906856]  __netif_receive_skb_one_core+0x67/0x70
> [165940.907639]  netif_receive_skb+0x35/0x110
> [165940.908295]  br_handle_frame_finish+0x17a/0x450 [bridge]
> [165940.909143]  ? ip_finish_output2+0x19b/0x560
> [165940.909842]  ? br_handle_frame_finish+0x450/0x450 [bridge]
> [165940.910718]  br_handle_frame+0x292/0x350 [bridge]
> [165940.911483]  ? ip_sublist_rcv_finish+0x57/0x70
> [165940.912199]  ? ___slab_alloc+0x127/0x5b0
> [165940.912835]  __netif_receive_skb_core+0x196/0xf70
> [165940.913590]  ? ip_list_rcv+0x125/0x140
> [165940.914201]  __netif_receive_skb_list_core+0x12f/0x2b0
> [165940.915024]  netif_receive_skb_list_internal+0x1bc/0x2e0
> [165940.915873]  ? vmxnet3_rq_rx_complete+0x8bd/0xde0 [vmxnet3]
> [165940.916769]  napi_complete_done+0x6f/0x190
> [165940.917439]  vmxnet3_poll_rx_only+0x7b/0xa0 [vmxnet3]
> [165940.918249]  net_rx_action+0x135/0x3b0
> [165940.918863]  __do_softirq+0xca/0x288
> [165940.919451]  asm_call_irq_on_stack+0xf/0x20
> [165940.920146]  </IRQ>
> [165940.920508]  do_softirq_own_stack+0x37/0x40
> [165940.921187]  irq_exit_rcu+0xc2/0x100
> [165940.921772]  common_interrupt+0x74/0x130
> [165940.922410]  asm_common_interrupt+0x1e/0x40

Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[Jozsef Kadlecsik]

On Wed, 10 Feb 2021, Reindl Harald wrote:

> > > why is that still not part of 5.10.14 given how old that issue is 
> > > 
> > > https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.10.14
> > 
> > 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

Re: [PATCH net 1/4] netfilter: xt_recent: Fix attempt to update deleted entry

[Jozsef Kadlecsik]

On Fri, 5 Feb 2021, Reindl Harald wrote:

> 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

[PATCH net 2/4] selftests: netfilter: fix current year

[Pablo Neira Ayuso]

From: Fabian Frederick <fabf@skynet.be>

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 <oliver.sang@intel.com>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 tools/testing/selftests/netfilter/nft_meta.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/netfilter/nft_meta.sh b/tools/testing/selftests/netfilter/nft_meta.sh
index 087f0e6e71ce..f33154c04d34 100755
--- a/tools/testing/selftests/netfilter/nft_meta.sh
+++ b/tools/testing/selftests/netfilter/nft_meta.sh
@@ -23,7 +23,7 @@ ip -net "$ns0" addr add 127.0.0.1 dev lo
 
 trap cleanup EXIT
 
-currentyear=$(date +%G)
+currentyear=$(date +%Y)
 lastyear=$((currentyear-1))
 ip netns exec "$ns0" nft -f /dev/stdin <<EOF
 table inet filter {
-- 
2.20.1

[PATCH net 3/4] netfilter: nftables: fix possible UAF over chains from packet path in netns

[Pablo Neira Ayuso]

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(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8d3aa97b52e7..43fe80f10313 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -8949,6 +8949,17 @@ int __nft_release_basechain(struct nft_ctx *ctx)
 }
 EXPORT_SYMBOL_GPL(__nft_release_basechain);
 
+static void __nft_release_hooks(struct net *net)
+{
+	struct nft_table *table;
+	struct nft_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);
+	}
+}
+
 static void __nft_release_tables(struct net *net)
 {
 	struct nft_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)
 	return 0;
 }
 
+static void __net_exit nf_tables_pre_exit_net(struct net *net)
+{
+	__nft_release_hooks(net);
+}
+
 static void __net_exit nf_tables_exit_net(struct net *net)
 {
 	mutex_lock(&net->nft.commit_mutex);
@@ -9029,8 +9041,9 @@ static void __net_exit nf_tables_exit_net(struct net *net)
 }
 
 static struct pernet_operations nf_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,
 };
 
 static int __init nf_tables_module_init(void)
-- 
2.20.1

[PATCH net 4/4] netfilter: flowtable: fix tcp and udp header checksum update

[Pablo Neira Ayuso]

From: Sven Auhagen <sven.auhagen@voleatech.de>

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 <sven.auhagen@voleatech.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_flow_table_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 513f78db3cb2..4a4acbba78ff 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -399,7 +399,7 @@ static int nf_flow_nat_port_tcp(struct sk_buff *skb, unsigned int thoff,
 		return -1;
 
 	tcph = (void *)(skb_network_header(skb) + thoff);
-	inet_proto_csum_replace2(&tcph->check, skb, port, new_port, true);
+	inet_proto_csum_replace2(&tcph->check, skb, port, new_port, false);
 
 	return 0;
 }
@@ -415,7 +415,7 @@ static int nf_flow_nat_port_udp(struct sk_buff *skb, unsigned int thoff,
 	udph = (void *)(skb_network_header(skb) + thoff);
 	if (udph->check || skb->ip_summed == CHECKSUM_PARTIAL) {
 		inet_proto_csum_replace2(&udph->check, skb, port,
-					 new_port, true);
+					 new_port, false);
 		if (!udph->check)
 			udph->check = CSUM_MANGLED_0;
 	}
-- 
2.20.1