An error occurred fetching the project authors.
  1. 03 Jan, 2024 2 commits
  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