1. 10 Jan, 2021 1 commit
    • Chen Yi's avatar
      selftests: netfilter: Pass family parameter "-f" to conntrack tool · fab336b4
      Chen Yi authored
      Fix nft_conntrack_helper.sh false fail report:
      
      1) Conntrack tool need "-f ipv6" parameter to show out ipv6 traffic items.
      
      2) Sleep 1 second after background nc send packet, to make sure check
      is after this statement executed.
      
      False report:
      FAIL: ns1-lkjUemYw did not show attached helper ip set via ruleset
      PASS: ns1-lkjUemYw connection on port 2121 has ftp helper attached
      ...
      
      After fix:
      PASS: ns1-2hUniwU2 connection on port 2121 has ftp helper attached
      PASS: ns2-2hUniwU2 connection on port 2121 has ftp helper attached
      ...
      
      Fixes: 619ae8e0 ("selftests: netfilter: add test case for conntrack helper assignment")
      Signed-off-by: default avatarChen Yi <yiche@redhat.com>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      fab336b4
  2. 09 Jan, 2021 8 commits
    • Jakub Kicinski's avatar
      Merge branch 'net-fix-issues-around-register_netdevice-failures' · c49243e8
      Jakub Kicinski authored
      Jakub Kicinski says:
      
      ====================
      net: fix issues around register_netdevice() failures
      
      This series attempts to clean up the life cycle of struct
      net_device. Dave has added dev->needs_free_netdev in the
      past to fix double frees, we can lean on that mechanism
      a little more to fix remaining issues with register_netdevice().
      
      This is the next chapter of the saga which already includes:
      commit 0e0eee24 ("net: correct error path in rtnl_newlink()")
      commit e51fb152 ("rtnetlink: fix a memory leak when ->newlink fails")
      commit cf124db5 ("net: Fix inconsistent teardown and release of private netdev state.")
      commit 93ee31f1 ("[NET]: Fix free_netdev on register_netdev failure.")
      commit 814152a8 ("net: fix memleak in register_netdevice()")
      commit 10cc514f ("net: Fix null de-reference of device refcount")
      
      The immediate problem which gets fixed here is that calling
      free_netdev() right after unregister_netdevice() is illegal
      because we need to release rtnl_lock first, to let the
      unregistration finish. Note that unregister_netdevice() is
      just a wrapper of unregister_netdevice_queue(), it only
      does half of the job.
      
      Where this limitation becomes most problematic is in failure
      modes of register_netdevice(). There is a notifier call right
      at the end of it, which lets other subsystems veto the entire
      thing. At which point we should really go through a full
      unregister_netdevice(), but we can't because callers may
      go straight to free_netdev() after the failure, and that's
      no bueno (see the previous paragraph).
      
      This set makes free_netdev() more lenient, when device
      is still being unregistered free_netdev() will simply set
      dev->needs_free_netdev and let the unregister process do
      the freeing.
      
      With the free_netdev() problem out of the way failures in
      register_netdevice() can make use of net_todo, again.
      Users are still expected to call free_netdev() right after
      failure but that will only set dev->needs_free_netdev.
      
      To prevent the pathological case of:
      
       dev->needs_free_netdev = true;
       if (register_netdevice(dev)) {
         rtnl_unlock();
         free_netdev(dev);
       }
      
      make register_netdevice()'s failure clear dev->needs_free_netdev.
      
      Problems described above are only present with register_netdevice() /
      unregister_netdevice(). We have two parallel APIs for registration
      of devices:
       - those called outside rtnl_lock (register_netdev(), and
         unregister_netdev());
       - and those to be used under rtnl_lock - register_netdevice()
         and unregister_netdevice().
      The former is trivial and has no problems. The alternative
      approach to fix the latter would be to also separate the
      freeing functions - i.e. add free_netdevice(). This has been
      implemented (incl. converting all relevant calls in the tree)
      but it feels a little unnecessary to put the burden of choosing
      the right free_netdev{,ice}() call on the programmer when we
      can "just do the right thing" by default.
      ====================
      
      Link: https://lore.kernel.org/r/20210106184007.1821480-1-kuba@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      c49243e8
    • Jakub Kicinski's avatar
      net: make sure devices go through netdev_wait_all_refs · 766b0515
      Jakub Kicinski authored
      If register_netdevice() fails at the very last stage - the
      notifier call - some subsystems may have already seen it and
      grabbed a reference. struct net_device can't be freed right
      away without calling netdev_wait_all_refs().
      
      Now that we have a clean interface in form of dev->needs_free_netdev
      and lenient free_netdev() we can undo what commit 93ee31f1 ("[NET]:
      Fix free_netdev on register_netdev failure.") has done and complete
      the unregistration path by bringing the net_set_todo() call back.
      
      After registration fails user is still expected to explicitly
      free the net_device, so make sure ->needs_free_netdev is cleared,
      otherwise rolling back the registration will cause the old double
      free for callers who release rtnl_lock before the free.
      
      This also solves the problem of priv_destructor not being called
      on notifier error.
      
      net_set_todo() will be moved back into unregister_netdevice_queue()
      in a follow up.
      Reported-by: default avatarHulk Robot <hulkci@huawei.com>
      Reported-by: default avatarYang Yingliang <yangyingliang@huawei.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      766b0515
    • Jakub Kicinski's avatar
      net: make free_netdev() more lenient with unregistering devices · c269a24c
      Jakub Kicinski authored
      There are two flavors of handling netdev registration:
       - ones called without holding rtnl_lock: register_netdev() and
         unregister_netdev(); and
       - those called with rtnl_lock held: register_netdevice() and
         unregister_netdevice().
      
      While the semantics of the former are pretty clear, the same can't
      be said about the latter. The netdev_todo mechanism is utilized to
      perform some of the device unregistering tasks and it hooks into
      rtnl_unlock() so the locked variants can't actually finish the work.
      In general free_netdev() does not mix well with locked calls. Most
      drivers operating under rtnl_lock set dev->needs_free_netdev to true
      and expect core to make the free_netdev() call some time later.
      
      The part where this becomes most problematic is error paths. There is
      no way to unwind the state cleanly after a call to register_netdevice(),
      since unreg can't be performed fully without dropping locks.
      
      Make free_netdev() more lenient, and defer the freeing if device
      is being unregistered. This allows error paths to simply call
      free_netdev() both after register_netdevice() failed, and after
      a call to unregister_netdevice() but before dropping rtnl_lock.
      
      Simplify the error paths which are currently doing gymnastics
      around free_netdev() handling.
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      c269a24c
    • Jakub Kicinski's avatar
      docs: net: explain struct net_device lifetime · 2b446e65
      Jakub Kicinski authored
      Explain the two basic flows of struct net_device's operation.
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      2b446e65
    • Tom Parkin's avatar
      ppp: fix refcount underflow on channel unbridge · c1787ffd
      Tom Parkin authored
      When setting up a channel bridge, ppp_bridge_channels sets the
      pch->bridge field before taking the associated reference on the bridge
      file instance.
      
      This opens up a refcount underflow bug if ppp_bridge_channels called
      via. iotcl runs concurrently with ppp_unbridge_channels executing via.
      file release.
      
      The bug is triggered by ppp_bridge_channels taking the error path
      through the 'err_unset' label.  In this scenario, pch->bridge is set,
      but the reference on the bridged channel will not be taken because
      the function errors out.  If ppp_unbridge_channels observes pch->bridge
      before it is unset by the error path, it will erroneously drop the
      reference on the bridged channel and cause a refcount underflow.
      
      To avoid this, ensure that ppp_bridge_channels holds a reference on
      each channel in advance of setting the bridge pointers.
      Signed-off-by: default avatarTom Parkin <tparkin@katalix.com>
      Fixes: 4cf476ce ("ppp: add PPPIOCBRIDGECHAN and PPPIOCUNBRIDGECHAN ioctls")
      Acked-by: default avatarGuillaume Nault <gnault@redhat.com>
      Link: https://lore.kernel.org/r/20210107181315.3128-1-tparkin@katalix.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      c1787ffd
    • Baptiste Lepers's avatar
      udp: Prevent reuseport_select_sock from reading uninitialized socks · fd2ddef0
      Baptiste Lepers authored
      reuse->socks[] is modified concurrently by reuseport_add_sock. To
      prevent reading values that have not been fully initialized, only read
      the array up until the last known safe index instead of incorrectly
      re-reading the last index of the array.
      
      Fixes: acdcecc6 ("udp: correct reuseport selection with connected sockets")
      Signed-off-by: default avatarBaptiste Lepers <baptiste.lepers@gmail.com>
      Acked-by: default avatarWillem de Bruijn <willemb@google.com>
      Link: https://lore.kernel.org/r/20210107051110.12247-1-baptiste.lepers@gmail.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      fd2ddef0
    • Dongseok Yi's avatar
      net: fix use-after-free when UDP GRO with shared fraglist · 53475c5d
      Dongseok Yi authored
      skbs in fraglist could be shared by a BPF filter loaded at TC. If TC
      writes, it will call skb_ensure_writable -> pskb_expand_head to create
      a private linear section for the head_skb. And then call
      skb_clone_fraglist -> skb_get on each skb in the fraglist.
      
      skb_segment_list overwrites part of the skb linear section of each
      fragment itself. Even after skb_clone, the frag_skbs share their
      linear section with their clone in PF_PACKET.
      
      Both sk_receive_queue of PF_PACKET and PF_INET (or PF_INET6) can have
      a link for the same frag_skbs chain. If a new skb (not frags) is
      queued to one of the sk_receive_queue, multiple ptypes can see and
      release this. It causes use-after-free.
      
      [ 4443.426215] ------------[ cut here ]------------
      [ 4443.426222] refcount_t: underflow; use-after-free.
      [ 4443.426291] WARNING: CPU: 7 PID: 28161 at lib/refcount.c:190
      refcount_dec_and_test_checked+0xa4/0xc8
      [ 4443.426726] pstate: 60400005 (nZCv daif +PAN -UAO)
      [ 4443.426732] pc : refcount_dec_and_test_checked+0xa4/0xc8
      [ 4443.426737] lr : refcount_dec_and_test_checked+0xa0/0xc8
      [ 4443.426808] Call trace:
      [ 4443.426813]  refcount_dec_and_test_checked+0xa4/0xc8
      [ 4443.426823]  skb_release_data+0x144/0x264
      [ 4443.426828]  kfree_skb+0x58/0xc4
      [ 4443.426832]  skb_queue_purge+0x64/0x9c
      [ 4443.426844]  packet_set_ring+0x5f0/0x820
      [ 4443.426849]  packet_setsockopt+0x5a4/0xcd0
      [ 4443.426853]  __sys_setsockopt+0x188/0x278
      [ 4443.426858]  __arm64_sys_setsockopt+0x28/0x38
      [ 4443.426869]  el0_svc_common+0xf0/0x1d0
      [ 4443.426873]  el0_svc_handler+0x74/0x98
      [ 4443.426880]  el0_svc+0x8/0xc
      
      Fixes: 3a1296a3 (net: Support GRO/GSO fraglist chaining.)
      Signed-off-by: default avatarDongseok Yi <dseok.yi@samsung.com>
      Acked-by: default avatarWillem de Bruijn <willemb@google.com>
      Acked-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Link: https://lore.kernel.org/r/1610072918-174177-1-git-send-email-dseok.yi@samsung.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      53475c5d
    • Stephan Gerhold's avatar
      net: ipa: modem: add missing SET_NETDEV_DEV() for proper sysfs links · afba9dc1
      Stephan Gerhold authored
      At the moment it is quite hard to identify the network interface
      provided by IPA in userspace components: The network interface is
      created as virtual device, without any link to the IPA device.
      The interface name ("rmnet_ipa%d") is the only indication that the
      network interface belongs to IPA, but this is not very reliable.
      
      Add SET_NETDEV_DEV() to associate the network interface with the
      IPA parent device. This allows userspace services like ModemManager
      to properly identify that this network interface is provided by IPA
      and belongs to the modem.
      
      Cc: Alex Elder <elder@kernel.org>
      Fixes: a646d6ec ("soc: qcom: ipa: modem and microcontroller")
      Signed-off-by: default avatarStephan Gerhold <stephan@gerhold.net>
      Link: https://lore.kernel.org/r/20210106100755.56800-1-stephan@gerhold.netSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      afba9dc1
  3. 08 Jan, 2021 25 commits
  4. 07 Jan, 2021 6 commits
    • Jakub Kicinski's avatar
      Merge https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf · 0565ff56
      Jakub Kicinski authored
      Alexei Starovoitov says:
      
      ====================
      pull-request: bpf 2021-01-07
      
      We've added 4 non-merge commits during the last 10 day(s) which contain
      a total of 4 files changed, 14 insertions(+), 7 deletions(-).
      
      The main changes are:
      
      1) Fix task_iter bug caused by the merge conflict resolution, from Yonghong.
      
      2) Fix resolve_btfids for multiple type hierarchies, from Jiri.
      
      * https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf:
        bpftool: Fix compilation failure for net.o with older glibc
        tools/resolve_btfids: Warn when having multiple IDs for single type
        bpf: Fix a task_iter bug caused by a merge conflict resolution
        selftests/bpf: Fix a compile error for BPF_F_BPRM_SECUREEXEC
      ====================
      
      Link: https://lore.kernel.org/r/20210107221555.64959-1-alexei.starovoitov@gmail.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      0565ff56
    • Jakub Kicinski's avatar
      Merge branch 'net-fix-netfilter-defrag-ip-tunnel-pmtu-blackhole' · 704a0f85
      Jakub Kicinski authored
      Florian Westphal says:
      
      ====================
      net: fix netfilter defrag/ip tunnel pmtu blackhole
      
      Christian Perle reported a PMTU blackhole due to unexpected interaction
      between the ip defragmentation that comes with connection tracking and
      ip tunnels.
      
      Unfortunately setting 'nopmtudisc' on the tunnel breaks the test
      scenario even without netfilter.
      
      Christinas setup looks like this:
           +--------+       +---------+       +--------+
           |Router A|-------|Wanrouter|-------|Router B|
           |        |.IPIP..|         |..IPIP.|        |
           +--------+       +---------+       +--------+
                /             mtu 1400           \
               /                                  \
       +--------+                                  +--------+
       |Client A|                                  |Client B|
       +--------+                                  +--------+
      
      MTU is 1500 everywhere, except on Router A to Wanrouter and
      Wanrouter to Router B.
      
      Router A and Router B use IPIP tunnel interfaces to tunnel traffic
      between Client A and Client B over WAN.
      
      Client A sends a 1400 byte UDP datagram to Client B.
      This packet gets encapsulated in the IPIP tunnel.
      
      This works, packet is received on client B.
      
      When conntrack (or anything else that forces ip defragmentation) is
      enabled on Router A, the packet gets dropped on Router A after
      encapsulation because they exceed the link MTU.
      
      Setting the 'nopmtudisc' flag on the IPIP tunnel makes things worse,
      no packets pass even in the no-netfilter scenario.
      
      Patch one is a reproducer script for selftest infra.
      
      Patch two is a fix for 'nopmtudisc' behaviour so ip_tunnel will send
      an icmp error to Client A.  This allows 'nopmtudisc' tunnel to forward
      the UDP datagrams.
      
      Patch three enables ip refragmentation for all reassembled packets, just
      like ipv6.
      ====================
      
      Link: https://lore.kernel.org/r/20210105231523.622-1-fw@strlen.deSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      704a0f85
    • Florian Westphal's avatar
      net: ip: always refragment ip defragmented packets · bb4cc1a1
      Florian Westphal authored
      Conntrack reassembly records the largest fragment size seen in IPCB.
      However, when this gets forwarded/transmitted, fragmentation will only
      be forced if one of the fragmented packets had the DF bit set.
      
      In that case, a flag in IPCB will force fragmentation even if the
      MTU is large enough.
      
      This should work fine, but this breaks with ip tunnels.
      Consider client that sends a UDP datagram of size X to another host.
      
      The client fragments the datagram, so two packets, of size y and z, are
      sent. DF bit is not set on any of these packets.
      
      Middlebox netfilter reassembles those packets back to single size-X
      packet, before routing decision.
      
      packet-size-vs-mtu checks in ip_forward are irrelevant, because DF bit
      isn't set.  At output time, ip refragmentation is skipped as well
      because x is still smaller than the mtu of the output device.
      
      If ttransmit device is an ip tunnel, the packet size increases to
      x+overhead.
      
      Also, tunnel might be configured to force DF bit on outer header.
      
      In this case, packet will be dropped (exceeds MTU) and an ICMP error is
      generated back to sender.
      
      But sender already respects the announced MTU, all the packets that
      it sent did fit the announced mtu.
      
      Force refragmentation as per original sizes unconditionally so ip tunnel
      will encapsulate the fragments instead.
      
      The only other solution I see is to place ip refragmentation in
      the ip_tunnel code to handle this case.
      
      Fixes: d6b915e2 ("ip_fragment: don't forward defragmented DF packet")
      Reported-by: default avatarChristian Perle <christian.perle@secunet.com>
      Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
      Acked-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      bb4cc1a1
    • Florian Westphal's avatar
      net: fix pmtu check in nopmtudisc mode · 50c66167
      Florian Westphal authored
      For some reason ip_tunnel insist on setting the DF bit anyway when the
      inner header has the DF bit set, EVEN if the tunnel was configured with
      'nopmtudisc'.
      
      This means that the script added in the previous commit
      cannot be made to work by adding the 'nopmtudisc' flag to the
      ip tunnel configuration. Doing so breaks connectivity even for the
      without-conntrack/netfilter scenario.
      
      When nopmtudisc is set, the tunnel will skip the mtu check, so no
      icmp error is sent to client. Then, because inner header has DF set,
      the outer header gets added with DF bit set as well.
      
      IP stack then sends an error to itself because the packet exceeds
      the device MTU.
      
      Fixes: 23a3647b ("ip_tunnels: Use skb-len to PMTU check.")
      Cc: Stefano Brivio <sbrivio@redhat.com>
      Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
      Acked-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      50c66167
    • Florian Westphal's avatar
      selftests: netfilter: add selftest for ipip pmtu discovery with enabled connection tracking · 9e7a67de
      Florian Westphal authored
      Convert Christians bug description into a reproducer.
      
      Cc: Shuah Khan <shuah@kernel.org>
      Reported-by: default avatarChristian Perle <christian.perle@secunet.com>
      Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
      Acked-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      9e7a67de
    • Lukas Bulwahn's avatar
      docs: octeontx2: tune rst markup · f3562f5e
      Lukas Bulwahn authored
      Commit 80b94148 ("docs: octeontx2: Add Documentation for NPA health
      reporters") added new documentation with improper formatting for rst, and
      caused a few new warnings for make htmldocs in octeontx2.rst:169--202.
      
      Tune markup and formatting for better presentation in the HTML view.
      Signed-off-by: default avatarLukas Bulwahn <lukas.bulwahn@gmail.com>
      Acked-by: default avatarRandy Dunlap <rdunlap@infradead.org>
      Acked-by: default avatarGeorge Cherian <george.cherian@marvell.com>
      Link: https://lore.kernel.org/r/20210106161735.21751-1-lukas.bulwahn@gmail.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      f3562f5e