1. 01 Jan, 2024 4 commits
  2. 29 Dec, 2023 3 commits
  3. 27 Dec, 2023 1 commit
  4. 26 Dec, 2023 1 commit
  5. 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
  6. 21 Dec, 2023 12 commits
  7. 20 Dec, 2023 13 commits
  8. 19 Dec, 2023 3 commits
    • Kent Overstreet's avatar
    • Kent Overstreet's avatar
    • Linus Torvalds's avatar
      Merge tag 'trace-v6.7-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace · 55cb5f43
      Linus Torvalds authored
      Pull tracing fix from Steven Rostedt:
       "While working on the ring buffer, I found one more bug with the
        timestamp code, and the fix for this removed the need for the final
        64-bit cmpxchg!
      
        The ring buffer events hold a "delta" from the previous event. If it
        is determined that the delta can not be calculated, it falls back to
        adding an absolute timestamp value. The way to know if the delta can
        be used is via two stored timestamps in the per-cpu buffer meta data:
      
         before_stamp and write_stamp
      
        The before_stamp is written by every event before it tries to allocate
        its space on the ring buffer. The write_stamp is written after it
        allocates its space and knows that nothing came in after it read the
        previous before_stamp and write_stamp and the two matched.
      
        A previous fix dd939425 ("ring-buffer: Do not try to put back
        write_stamp") removed putting back the write_stamp to match the
        before_stamp so that the next event could use the delta, but races
        were found where the two would match, but not be for of the previous
        event.
      
        It was determined to allow the event reservation to not have a valid
        write_stamp when it is finished, and this fixed a lot of races.
      
        The last use of the 64-bit timestamp cmpxchg depended on the
        write_stamp being valid after an interruption. But this is no longer
        the case, as if an event is interrupted by a softirq that writes an
        event, and that event gets interrupted by a hardirq or NMI and that
        writes an event, then the softirq could finish its reservation without
        a valid write_stamp.
      
        In the slow path of the event reservation, a delta can still be used
        if the write_stamp is valid. Instead of using a cmpxchg against the
        write stamp, the before_stamp needs to be read again to validate the
        write_stamp. The cmpxchg is not needed.
      
        This updates the slowpath to validate the write_stamp by comparing it
        to the before_stamp and removes all rb_time_cmpxchg() as there are no
        more users of that function.
      
        The removal of the 32-bit updates of rb_time_t will be done in the
        next merge window"
      
      * tag 'trace-v6.7-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace:
        ring-buffer: Fix slowpath of interrupted event
      55cb5f43