[PATCH net v2] nfc: llcp: fix NULL pointer dereference race in nfc_llcp_send_ui_frame()

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 a reference to sock->local under the socket lock and pass it to
nfc_llcp_send_ui_frame(), which now uses the pinned local->dev instead
of re-reading sock->dev. nfc_llcp_local_get() also pins the nfc_dev, so
both stay valid against a concurrent llcp_sock_bind().

Fixes: dded08927ca3 ("nfc: llcp: fix NULL error pointer dereference on sendmsg() after failed bind()")
Signed-off-by: Junwoong Doh <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/
---
v2:
  - Changed locking to refcounting to avoid holding the socket lock
    across the blocking send.

v1: https://lore.kernel.org/all/[email protected]/

 net/nfc/llcp.h          |  4 +++-
 net/nfc/llcp_commands.c | 10 +++-------
 net/nfc/llcp_core.c     |  2 +-
 net/nfc/llcp_sock.c     | 10 ++++++++--
 4 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/net/nfc/llcp.h b/net/nfc/llcp.h
index d8345ed57c95..3ff9729daf63 100644
--- a/net/nfc/llcp.h
+++ b/net/nfc/llcp.h
@@ -201,6 +201,7 @@ void nfc_llcp_sock_link(struct llcp_sock_list *l, struct sock *s);
 void nfc_llcp_sock_unlink(struct llcp_sock_list *l, struct sock *s);
 void nfc_llcp_socket_remote_param_init(struct nfc_llcp_sock *sock);
 struct nfc_llcp_local *nfc_llcp_find_local(struct nfc_dev *dev);
+struct nfc_llcp_local *nfc_llcp_local_get(struct nfc_llcp_local *local);
 int nfc_llcp_local_put(struct nfc_llcp_local *local);
 u8 nfc_llcp_get_sdp_ssap(struct nfc_llcp_local *local,
 			 struct nfc_llcp_sock *sock);
@@ -243,7 +244,8 @@ int nfc_llcp_send_dm(struct nfc_llcp_local *local, u8 ssap, u8 dsap, u8 reason);
 int nfc_llcp_send_disconnect(struct nfc_llcp_sock *sock);
 int nfc_llcp_send_i_frame(struct nfc_llcp_sock *sock,
 			  struct msghdr *msg, size_t len);
-int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
+int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock,
+			   struct nfc_llcp_local *local, u8 ssap, u8 dsap,
 			   struct msghdr *msg, size_t len);
 int nfc_llcp_send_rr(struct nfc_llcp_sock *sock);
 
diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index ca89fe967d6a..1549ed2631f5 100644
--- a/net/nfc/llcp_commands.c
+++ b/net/nfc/llcp_commands.c
@@ -740,11 +740,11 @@ int nfc_llcp_send_i_frame(struct nfc_llcp_sock *sock,
 	return len;
 }
 
-int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
+int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock,
+			   struct nfc_llcp_local *local, u8 ssap, u8 dsap,
 			   struct msghdr *msg, size_t len)
 {
 	struct sk_buff *pdu;
-	struct nfc_llcp_local *local;
 	size_t frag_len = 0, remaining_len;
 	u8 *msg_ptr, *msg_data;
 	u16 remote_miu;
@@ -752,10 +752,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;
@@ -777,7 +773,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(local->dev, &sock->sk, 0,
 					 frag_len + LLCP_HEADER_SIZE, &err);
 		if (pdu == NULL) {
 			pr_err("Could not allocate PDU (error=%d)\n", err);
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index cac1b5487064..52e72399d731 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -143,7 +143,7 @@ static void nfc_llcp_socket_release(struct nfc_llcp_local *local, bool device,
 	write_unlock(&local->raw_sockets.lock);
 }
 
-static struct nfc_llcp_local *nfc_llcp_local_get(struct nfc_llcp_local *local)
+struct nfc_llcp_local *nfc_llcp_local_get(struct nfc_llcp_local *local)
 {
 	/* Since using nfc_llcp_local may result in usage of nfc_dev, whenever
 	 * we hold a reference to local, we also need to hold a reference to
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index 5558d8a4d48b..4d38025e8213 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -787,6 +787,7 @@ static int llcp_sock_sendmsg(struct socket *sock, struct msghdr *msg,
 {
 	struct sock *sk = sock->sk;
 	struct nfc_llcp_sock *llcp_sock = nfc_llcp_sock(sk);
+	struct nfc_llcp_local *local;
 	int ret;
 
 	pr_debug("sock %p sk %p", sock, sk);
@@ -819,10 +820,15 @@ static int llcp_sock_sendmsg(struct socket *sock, struct msghdr *msg,
 			return -EINVAL;
 		}
 
+		local = nfc_llcp_local_get(llcp_sock->local);
 		release_sock(sk);
+		if (!local)
+			return -ENODEV;
 
-		return nfc_llcp_send_ui_frame(llcp_sock, addr->dsap, addr->ssap,
-					      msg, len);
+		ret = nfc_llcp_send_ui_frame(llcp_sock, local, addr->dsap,
+					     addr->ssap, msg, len);
+		nfc_llcp_local_put(local);
+		return ret;
 	}
 
 	if (sk->sk_state != LLCP_CONNECTED) {
-- 
2.34.1