1. 03 Jan, 2024 1 commit
    • Brad Cowie's avatar
      netfilter: nf_nat: fix action not being set for all ct states · e6345d28
      Brad Cowie authored
      This fixes openvswitch's handling of nat packets in the related state.
      
      In nf_ct_nat_execute(), which is called from nf_ct_nat(), ICMP/ICMPv6
      packets in the IP_CT_RELATED or IP_CT_RELATED_REPLY state, which have
      not been dropped, will follow the goto, however the placement of the
      goto label means that updating the action bit field will be bypassed.
      
      This causes ovs_nat_update_key() to not be called from ovs_ct_nat()
      which means the openvswitch match key for the ICMP/ICMPv6 packet is not
      updated and the pre-nat value will be retained for the key, which will
      result in the wrong openflow rule being matched for that packet.
      
      Move the goto label above where the action bit field is being set so
      that it is updated in all cases where the packet is accepted.
      
      Fixes: ebddb140 ("net: move the nat function to nf_nat_ovs for ovs and tc")
      Signed-off-by: default avatarBrad Cowie <brad@faucet.nz>
      Reviewed-by: default avatarSimon Horman <horms@kernel.org>
      Acked-by: default avatarXin Long <lucien.xin@gmail.com>
      Acked-by: default avatarAaron Conole <aconole@redhat.com>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      e6345d28
  2. 01 Jan, 2024 5 commits
  3. 29 Dec, 2023 3 commits
  4. 27 Dec, 2023 1 commit
  5. 26 Dec, 2023 1 commit
  6. 25 Dec, 2023 3 commits
    • David S. Miller's avatar
      Merge branch 'nfc-refcounting' · dff90e4a
      David S. Miller authored
      @ 2023-12-19 17:49 Siddh Raman Pant
        2023-12-19 17:49 ` [PATCH net-next v7 1/2] nfc: llcp_core: Hold a ref to llcp_local->dev when holding a ref to llcp_local Siddh Raman Pant
        2023-12-19 17:49 ` [PATCH net-next v7 2/2] nfc: Do not send datagram if socket state isn't LLCP_BOUND Siddh Raman Pant
        0 siblings, 2 replies; 4+ messages in thread
      Siddh Raman Pant says:
      
      ====================
      [PATCH net-next v7 0/2] nfc: Fix UAF during datagram sending caused by missing refcounting
      
      Changes in v7:
      - Stupidly reverted ordering in recv() too, fix that.
      - Remove redundant call to nfc_llcp_sock_free().
      
      Changes in v6:
      - Revert label introduction from v4, and thus also v5 entirely.
      
      Changes in v5:
      - Move reason = LLCP_DM_REJ under the fail_put_sock label.
      - Checkpatch now warns about == NULL check for new_sk, so fix that,
        and also at other similar places in the same function.
      
      Changes in v4:
      - Fix put ordering and comments.
      - Separate freeing in recv() into end labels.
      - Remove obvious comment and add reasoning.
      - Picked up r-bs by Suman.
      
      Changes in v3:
      - Fix missing freeing statements.
      
      Changes in v2:
      - Add net-next in patch subject.
      - Removed unnecessary extra lock and hold nfc_dev ref when holding llcp_sock.
      - Remove last formatting patch.
      - Picked up r-b from Krzysztof for LLCP_BOUND patch.
      
      ---
      
      For connectionless transmission, llcp_sock_sendmsg() codepath will
      eventually call nfc_alloc_send_skb() which takes in an nfc_dev as
      an argument for calculating the total size for skb allocation.
      
      virtual_ncidev_close() codepath eventually releases socket by calling
      nfc_llcp_socket_release() (which sets the sk->sk_state to LLCP_CLOSED)
      and afterwards the nfc_dev will be eventually freed.
      
      When an ndev gets freed, llcp_sock_sendmsg() will result in an
      use-after-free as it
      
      (1) doesn't have any checks in place for avoiding the datagram sending.
      
      (2) calls nfc_llcp_send_ui_frame(), which also has a do-while loop
          which can race with freeing. This loop contains the call to
          nfc_alloc_send_skb() where we dereference the nfc_dev pointer.
      
      nfc_dev is being freed because we do not hold a reference to it when
      we hold a reference to llcp_local. Thus, virtual_ncidev_close()
      eventually calls nfc_release() due to refcount going to 0.
      
      Since state has to be LLCP_BOUND for datagram sending, we can bail out
      early in llcp_sock_sendmsg().
      
      Please review and let me know if any errors are there, and hopefully
      this gets accepted.
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      dff90e4a
    • Siddh Raman Pant's avatar
      nfc: Do not send datagram if socket state isn't LLCP_BOUND · 6ec0d752
      Siddh Raman Pant authored
      As we know we cannot send the datagram (state can be set to LLCP_CLOSED
      by nfc_llcp_socket_release()), there is no need to proceed further.
      
      Thus, bail out early from llcp_sock_sendmsg().
      Signed-off-by: default avatarSiddh Raman Pant <code@siddh.me>
      Reviewed-by: default avatarKrzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
      Reviewed-by: default avatarSuman Ghosh <sumang@marvell.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      6ec0d752
    • Siddh Raman Pant's avatar
      nfc: llcp_core: Hold a ref to llcp_local->dev when holding a ref to llcp_local · c95f9195
      Siddh Raman Pant authored
      llcp_sock_sendmsg() calls nfc_llcp_send_ui_frame() which in turn calls
      nfc_alloc_send_skb(), which accesses the nfc_dev from the llcp_sock for
      getting the headroom and tailroom needed for skb allocation.
      
      Parallelly the nfc_dev can be freed, as the refcount is decreased via
      nfc_free_device(), leading to a UAF reported by Syzkaller, which can
      be summarized as follows:
      
      (1) llcp_sock_sendmsg() -> nfc_llcp_send_ui_frame()
      	-> nfc_alloc_send_skb() -> Dereference *nfc_dev
      (2) virtual_ncidev_close() -> nci_free_device() -> nfc_free_device()
      	-> put_device() -> nfc_release() -> Free *nfc_dev
      
      When a reference to llcp_local is acquired, we do not acquire the same
      for the nfc_dev. This leads to freeing even when the llcp_local is in
      use, and this is the case with the UAF described above too.
      
      Thus, when we acquire a reference to llcp_local, we should acquire a
      reference to nfc_dev, and release the references appropriately later.
      
      References for llcp_local is initialized in nfc_llcp_register_device()
      (which is called by nfc_register_device()). Thus, we should acquire a
      reference to nfc_dev there.
      
      nfc_unregister_device() calls nfc_llcp_unregister_device() which in
      turn calls nfc_llcp_local_put(). Thus, the reference to nfc_dev is
      appropriately released later.
      
      Reported-and-tested-by: syzbot+bbe84a4010eeea00982d@syzkaller.appspotmail.com
      Closes: https://syzkaller.appspot.com/bug?extid=bbe84a4010eeea00982d
      Fixes: c7aa1225 ("NFC: Take a reference on the LLCP local pointer when creating a socket")
      Reviewed-by: default avatarSuman Ghosh <sumang@marvell.com>
      Signed-off-by: default avatarSiddh Raman Pant <code@siddh.me>
      Reviewed-by: default avatarKrzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      c95f9195
  7. 21 Dec, 2023 12 commits
  8. 20 Dec, 2023 13 commits
  9. 19 Dec, 2023 1 commit