nfc_llcp_send_ui_frame() checks whether sock->local is NULL, but it is called by llcp_sock_sendmsg() without the socket lock held, which opens a window for a race condition. Between the sock->local check and the sock->dev use in nfc_alloc_send_skb(), llcp_sock_bind() can run concurrently and set both sock->local and sock->dev to NULL, which can lead to a NULL pointer dereference in nfc_alloc_send_skb(). Take the socket lock in nfc_llcp_send_ui_frame() so that the sock->local check and the sock->dev use are performed under it. The message is copied from user space before the lock is taken, to avoid holding the lock across a user space access that can block for an unbounded amount of time. Fixes: dded08927ca3 ("nfc: llcp: fix NULL error pointer dereference on sendmsg() after failed bind()") Signed-off-by: Junwoong Doh Link: https://lore.kernel.org/all/a89d0419-8bcf-40a2-b52d-3e5d911f11da@gmail.com/ --- net/nfc/llcp_commands.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c index 291f26facbf3..cfd5f6aebf8d 100644 --- a/net/nfc/llcp_commands.c +++ b/net/nfc/llcp_commands.c @@ -730,6 +730,7 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap, struct msghdr *msg, size_t len) { struct sk_buff *pdu; + struct sock *sk = &sock->sk; struct nfc_llcp_local *local; size_t frag_len = 0, remaining_len; u8 *msg_ptr, *msg_data; @@ -738,10 +739,6 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap, pr_debug("Send UI frame len %zd\n", len); - local = sock->local; - if (local == NULL) - return -ENODEV; - msg_data = kmalloc(len, GFP_USER | __GFP_NOWARN); if (msg_data == NULL) return -ENOMEM; @@ -751,6 +748,15 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap, return -EFAULT; } + lock_sock(sk); + + local = sock->local; + if (local == NULL) { + release_sock(sk); + kfree(msg_data); + return -ENODEV; + } + remaining_len = len; msg_ptr = msg_data; @@ -763,7 +769,7 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap, pr_debug("Fragment %zd bytes remaining %zd", frag_len, remaining_len); - pdu = nfc_alloc_send_skb(sock->dev, &sock->sk, 0, + pdu = nfc_alloc_send_skb(sock->dev, sk, 0, frag_len + LLCP_HEADER_SIZE, &err); if (pdu == NULL) { pr_err("Could not allocate PDU (error=%d)\n", err); @@ -800,6 +806,7 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap, msg_ptr += frag_len; } while (remaining_len > 0); + release_sock(sk); kfree(msg_data); return len; -- 2.34.1