Skip to content
  1. May 15, 2020
  3. May 14, 2020
  5. May 13, 2020
  7. May 12, 2020
  9. May 11, 2020
    • Florian Westphal's avatar
      netfilter: conntrack: fix infinite loop on rmmod · 54ab49fd
      Florian Westphal authored 
      

'rmmod nf_conntrack' can hang forever, because the netns exit
gets stuck in nf_conntrack_cleanup_net_list():

i_see_dead_people:
 busy = 0;
 list_for_each_entry(net, net_exit_list, exit_list) {
  nf_ct_iterate_cleanup(kill_all, net, 0, 0);
  if (atomic_read(&net->ct.count) != 0)
   busy = 1;
 }
 if (busy) {
  schedule();
  goto i_see_dead_people;
 }

When nf_ct_iterate_cleanup iterates the conntrack table, all nf_conn
structures can be found twice:
once for the original tuple and once for the conntracks reply tuple.

get_next_corpse() only calls the iterator when the entry is
in original direction -- the idea was to avoid unneeded invocations
of the iterator callback.

When support for clashing entries was added, the assumption that
all nf_conn objects are added twice, once in original, once for reply
tuple no longer holds -- NF_CLASH_BIT entries are only added in
the non-clashing reply direction.

Thus, if at least one NF_CLASH entry is in the list then
nf_conntrack_cleanup_net_list() always skips it completely.

During normal netns destruction, this causes a hang of several
seconds, until the gc worker removes the entry (NF_CLASH entries
always have a 1 second timeout).

But in the rmmod case, the gc worker has already been stopped, so
ct.count never becomes 0.

We can fix this in two ways:

1. Add a second test for CLASH_BIT and call iterator for those
   entries as well, or:
2. Skip the original tuple direction and use the reply tuple.

2) is simpler, so do that.

Fixes: 6a757c07 ("netfilter: conntrack: allow insertion of clashing entries")
Reported-by: default avatarChen Yi <yiche@redhat.com>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      54ab49fd
    • Roi Dayan's avatar
      netfilter: flowtable: Remove WQ_MEM_RECLAIM from workqueue · 1d10da0e
      Roi Dayan authored 
      

This workqueue is in charge of handling offloaded flow tasks like
add/del/stats we should not use WQ_MEM_RECLAIM flag.
The flag can result in the following warning.

[  485.557189] ------------[ cut here ]------------
[  485.562976] workqueue: WQ_MEM_RECLAIM nf_flow_table_offload:flow_offload_worr
[  485.562985] WARNING: CPU: 7 PID: 3731 at kernel/workqueue.c:2610 check_flush0
[  485.590191] Kernel panic - not syncing: panic_on_warn set ...
[  485.597100] CPU: 7 PID: 3731 Comm: kworker/u112:8 Not tainted 5.7.0-rc1.21802
[  485.606629] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.4.3 01/177
[  485.615487] Workqueue: nf_flow_table_offload flow_offload_work_handler [nf_f]
[  485.624834] Call Trace:
[  485.628077]  dump_stack+0x50/0x70
[  485.632280]  panic+0xfb/0x2d7
[  485.636083]  ? check_flush_dependency+0x110/0x130
[  485.641830]  __warn.cold.12+0x20/0x2a
[  485.646405]  ? check_flush_dependency+0x110/0x130
[  485.652154]  ? check_flush_dependency+0x110/0x130
[  485.657900]  report_bug+0xb8/0x100
[  485.662187]  ? sched_clock_cpu+0xc/0xb0
[  485.666974]  do_error_trap+0x9f/0xc0
[  485.671464]  do_invalid_op+0x36/0x40
[  485.675950]  ? check_flush_dependency+0x110/0x130
[  485.681699]  invalid_op+0x28/0x30

Fixes: 7da182a9 ("netfilter: flowtable: Use work entry per offload command")
Reported-by: default avatarMarcelo Ricardo Leitner <mleitner@redhat.com>
Signed-off-by: default avatarRoi Dayan <roid@mellanox.com>
Reviewed-by: default avatarPaul Blakey <paulb@mellanox.com>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      1d10da0e
    • Paul Blakey's avatar
      netfilter: flowtable: Add pending bit for offload work · 2c889795
      Paul Blakey authored 
      

Gc step can queue offloaded flow del work or stats work.
Those work items can race each other and a flow could be freed
before the stats work is executed and querying it.
To avoid that, add a pending bit that if a work exists for a flow
don't queue another work for it.
This will also avoid adding multiple stats works in case stats work
didn't complete but gc step started again.

Signed-off-by: default avatarPaul Blakey <paulb@mellanox.com>
Reviewed-by: default avatarRoi Dayan <roid@mellanox.com>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      2c889795
    • Luo bin's avatar
      hinic: fix a bug of ndo_stop · e8a1b0ef
      Luo bin authored 
      

if some function in ndo_stop interface returns failure because of
hardware fault, must go on excuting rest steps rather than return
failure directly, otherwise will cause memory leak.And bump the
timeout for SET_FUNC_STATE to ensure that cmd won't return failure
when hw is busy. Otherwise hw may stomp host memory if we free
memory regardless of the return value of SET_FUNC_STATE.

Fixes: 51ba902a ("net-next/hinic: Initialize hw interface")
Signed-off-by: default avatarLuo bin <luobin9@huawei.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      e8a1b0ef
    • Arnd Bergmann's avatar
      netfilter: conntrack: avoid gcc-10 zero-length-bounds warning · 2c407aca
      Arnd Bergmann authored 
      

gcc-10 warns around a suspicious access to an empty struct member:

net/netfilter/nf_conntrack_core.c: In function '__nf_conntrack_alloc':
net/netfilter/nf_conntrack_core.c:1522:9: warning: array subscript 0 is outside the bounds of an interior zero-length array 'u8[0]' {aka 'unsigned char[0]'} [-Wzero-length-bounds]
 1522 |  memset(&ct->__nfct_init_offset[0], 0,
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from net/netfilter/nf_conntrack_core.c:37:
include/net/netfilter/nf_conntrack.h:90:5: note: while referencing '__nfct_init_offset'
   90 |  u8 __nfct_init_offset[0];
      |     ^~~~~~~~~~~~~~~~~~

The code is correct but a bit unusual. Rework it slightly in a way that
does not trigger the warning, using an empty struct instead of an empty
array. There are probably more elegant ways to do this, but this is the
smallest change.

Fixes: c41884ce ("netfilter: conntrack: avoid zeroing timer")
Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      2c407aca
    • Florian Fainelli's avatar
      net: dsa: loop: Add module soft dependency · 3047211c
      Florian Fainelli authored 
      

There is a soft dependency against dsa_loop_bdinfo.ko which sets up the
MDIO device registration, since there are no symbols referenced by
dsa_loop.ko, there is no automatic loading of dsa_loop_bdinfo.ko which
is needed.

Fixes: 98cd1552 ("net: dsa: Mock-up driver")
Signed-off-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      3047211c
  11. May 10, 2020
    • Zefan Li's avatar
      netprio_cgroup: Fix unlimited memory leak of v2 cgroups · 090e28b2
      Zefan Li authored 
      

If systemd is configured to use hybrid mode which enables the use of
both cgroup v1 and v2, systemd will create new cgroup on both the default
root (v2) and netprio_cgroup hierarchy (v1) for a new session and attach
task to the two cgroups. If the task does some network thing then the v2
cgroup can never be freed after the session exited.

One of our machines ran into OOM due to this memory leak.

In the scenario described above when sk_alloc() is called
cgroup_sk_alloc() thought it's in v2 mode, so it stores
the cgroup pointer in sk->sk_cgrp_data and increments
the cgroup refcnt, but then sock_update_netprioidx()
thought it's in v1 mode, so it stores netprioidx value
in sk->sk_cgrp_data, so the cgroup refcnt will never be freed.

Currently we do the mode switch when someone writes to the ifpriomap
cgroup control file. The easiest fix is to also do the switch when
a task is attached to a new cgroup.

Fixes: bd1060a1 ("sock, cgroup: add sock->sk_cgroup")
Reported-by: default avatarYang Yingliang <yangyingliang@huawei.com>
Tested-by: default avatarYang Yingliang <yangyingliang@huawei.com>
Signed-off-by: default avatarZefan Li <lizefan@huawei.com>
Acked-by: default avatarTejun Heo <tj@kernel.org>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      090e28b2
    • Arnd Bergmann's avatar
      net: freescale: select CONFIG_FIXED_PHY where needed · 99352c79
      Arnd Bergmann authored 
      

I ran into a randconfig build failure with CONFIG_FIXED_PHY=m
and CONFIG_GIANFAR=y:

x86_64-linux-ld: drivers/net/ethernet/freescale/gianfar.o:(.rodata+0x418): undefined reference to `fixed_phy_change_carrier'

It seems the same thing can happen with dpaa and ucc_geth, so change
all three to do an explicit 'select FIXED_PHY'.

The fixed-phy driver actually has an alternative stub function that
theoretically allows building network drivers when fixed-phy is
disabled, but I don't see how that would help here, as the drivers
presumably would not work then.

Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
Acked-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      99352c79
  13. May 09, 2020
    • Paolo Abeni's avatar
      net: ipv4: really enforce backoff for redirects · 57644431
      Paolo Abeni authored 
      

In commit b406472b ("net: ipv4: avoid mixed n_redirects and
rate_tokens usage") I missed the fact that a 0 'rate_tokens' will
bypass the backoff algorithm.

Since rate_tokens is cleared after a redirect silence, and never
incremented on redirects, if the host keeps receiving packets
requiring redirect it will reply ignoring the backoff.

Additionally, the 'rate_last' field will be updated with the
cadence of the ingress packet requiring redirect. If that rate is
high enough, that will prevent the host from generating any
other kind of ICMP messages

The check for a zero 'rate_tokens' value was likely a shortcut
to avoid the more complex backoff algorithm after a redirect
silence period. Address the issue checking for 'n_redirects'
instead, which is incremented on successful redirect, and
does not interfere with other ICMP replies.

Fixes: b406472b ("net: ipv4: avoid mixed n_redirects and rate_tokens usage")
Reported-and-tested-by: default avatarColin Walters <walters@redhat.com>
Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      57644431
    • Wei Yongjun's avatar
      octeontx2-vf: Fix error return code in otx2vf_probe() · 9302bead
      Wei Yongjun authored 
      

Fix to return negative error code -ENOMEM from the alloc failed error
handling case instead of 0, as done elsewhere in this function.

Fixes: 3184fb5b ("octeontx2-vf: Virtual function driver support")
Reported-by: default avatarHulk Robot <hulkci@huawei.com>
Signed-off-by: default avatarWei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      9302bead