Fix UAF caused by racing datagram sending and freeing of nfc_dev
Get series with b4
b4 am [email protected]Versions newest first
- v1 Latest4 patches 7 repliesIn review
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/4 | nfc: Extract nfc_dev access from nfc_alloc_send_skb() into the callersinclude/ net/ net/ +1 more | +30 | 2 replies | |
| 2/4 | nfc: Protect access to nfc_dev in an llcp_sock with a rwlocknet/ net/ net/ +1 more | +46 | 2 replies | |
| 3/4 | nfc: Do not send datagram if socket state isn't LLCP_BOUNDnet/ | +5 | 1 reply | |
| 4/4 | nfc: llcp_sock_sendmsg: Reformat code to make the smaller block indentednet/ | +15 | 1 reply |
Reviews trailers per patch
1 of 4 patches has a Reviewed-by or Acked-by.
| Patch | [email protected] | Krzysztof Kozlowski |
|---|---|---|
| 1/4 | ||
| 2/4 | Reported-and-tested-by | |
| 3/4 | R-b | |
| 4/4 |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.
Cover letter [PATCH 0/4] Fix UAF caused by racing datagram sending and freeing of nfc_dev
(This patchset should be applied in order as changes are dependent.)
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 (a msg with size of 4096 is sent in chunks of 128 in this repro). This loop contains the call to nfc_alloc_send_skb().
Thus, I extracted nfc_dev access from nfc_alloc_send_skb() to the caller, and used a spinlock (rwlock) to protect/serialize access to the nfc_dev to avoid the UAF. Tested with syzkaller.
Since state has to be LLCP_BOUND for datagram sending, we can bail out early in llcp_sock_sendmsg().
The last patch is just a code reformat. As the datagram sending is a bigger chunk of the code, we can reduce unnecessary indentation.
Please review and let me know if any errors are there, and hopefully this gets accepted.
Thanks,
Siddh
Siddh Raman Pant (4):
nfc: Extract nfc_dev access from nfc_alloc_send_skb() into the callers
nfc: Protect access to nfc_dev in an llcp_sock with a rwlock
nfc: Do not send datagram if socket state isn't LLCP_BOUND
nfc: llcp_sock_sendmsg: Reformat code to make the smaller block
indented
include/net/nfc/nfc.h | 6 +++--- net/nfc/core.c | 14 +++++++------- net/nfc/llcp.h | 1 + net/nfc/llcp_commands.c | 41 +++++++++++++++++++++++++++++++++++------ net/nfc/llcp_core.c | 31 +++++++++++++++++++------------ net/nfc/llcp_sock.c | 31 ++++++++++++++++++------------- net/nfc/rawsock.c | 8 ++++++-- 7 files changed, 89 insertions(+), 43 deletions(-)
Signature · 1 line
2.42.0