Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
From:   Siddh Raman Pant <code@siddh.me>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 1/4] nfc: Extract nfc_dev access from nfc_alloc_send_skb() into the callers
Date:   Sun, 26 Nov 2023 01:56:16 +0530
Message-ID: <ba18da37e48b5c473e5b8bd76d6460017342f968.1700943019.git.code@siddh.me>
In-Reply-To: <cover.1700943019.git.code@siddh.me>
References: <cover.1700943019.git.code@siddh.me>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

The only reason why nfc_dev was accessed inside nfc_alloc_send_skb() is
for getting the headroom and tailroom values.

This can cause UAF to be reported from nfc_alloc_send_skb(), but the
callers are responsible for managing the device access, and thus the
UAF being reported, as the callers (like nfc_llcp_send_ui_frame()) may
repeatedly call this function, and this function will repeatedly try
to get the same headroom and tailroom values.

Thus, put the nfc_dev access responsibility on the callers and accept
the headroom and tailroom values directly.

Signed-off-by: Siddh Raman Pant <code@siddh.me>
---
 include/net/nfc/nfc.h   |  6 +++---
 net/nfc/core.c          | 14 +++++++-------
 net/nfc/llcp_commands.c | 20 ++++++++++++++------
 net/nfc/rawsock.c       |  8 ++++++--
 4 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/include/net/nfc/nfc.h b/include/net/nfc/nfc.h
index 5dee575fbe86..efe20a9a8499 100644
--- a/include/net/nfc/nfc.h
+++ b/include/net/nfc/nfc.h
@@ -260,9 +260,9 @@ static inline const char *nfc_device_name(const struct nfc_dev *dev)
 	return dev_name(&dev->dev);
 }
 
-struct sk_buff *nfc_alloc_send_skb(struct nfc_dev *dev, struct sock *sk,
-				   unsigned int flags, unsigned int size,
-				   unsigned int *err);
+struct sk_buff *nfc_alloc_send_skb(struct sock *sk, unsigned int flags,
+				   unsigned int size, int headroom,
+				   int tailroom, unsigned int *err);
 struct sk_buff *nfc_alloc_recv_skb(unsigned int size, gfp_t gfp);
 
 int nfc_set_remote_general_bytes(struct nfc_dev *dev,
diff --git a/net/nfc/core.c b/net/nfc/core.c
index eb2c0959e5b6..1f7d20971f6f 100644
--- a/net/nfc/core.c
+++ b/net/nfc/core.c
@@ -705,25 +705,25 @@ EXPORT_SYMBOL(nfc_tm_deactivated);
 /**
  * nfc_alloc_send_skb - allocate a skb for data exchange responses
  *
- * @dev: device sending the response
  * @sk: socket sending the response
  * @flags: MSG_DONTWAIT flag
  * @size: size to allocate
+ * @headroom: Extra headroom, in addition to size
+ * @tailroom: Extra tailroom, in addition to size
  * @err: pointer to memory to store the error code
  */
-struct sk_buff *nfc_alloc_send_skb(struct nfc_dev *dev, struct sock *sk,
-				   unsigned int flags, unsigned int size,
-				   unsigned int *err)
+struct sk_buff *nfc_alloc_send_skb(struct sock *sk, unsigned int flags,
+				   unsigned int size, int headroom,
+				   int tailroom, unsigned int *err)
 {
 	struct sk_buff *skb;
 	unsigned int total_size;
 
-	total_size = size +
-		dev->tx_headroom + dev->tx_tailroom + NFC_HEADER_SIZE;
+	total_size = size + headroom + tailroom + NFC_HEADER_SIZE;
 
 	skb = sock_alloc_send_skb(sk, total_size, flags & MSG_DONTWAIT, err);
 	if (skb)
-		skb_reserve(skb, dev->tx_headroom + NFC_HEADER_SIZE);
+		skb_reserve(skb, headroom + NFC_HEADER_SIZE);
 
 	return skb;
 }
diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index e2680a3bef79..39c7c59bbf66 100644
--- a/net/nfc/llcp_commands.c
+++ b/net/nfc/llcp_commands.c
@@ -314,13 +314,17 @@ static struct sk_buff *llcp_allocate_pdu(struct nfc_llcp_sock *sock,
 					 u8 cmd, u16 size)
 {
 	struct sk_buff *skb;
-	int err;
+	int err, headroom, tailroom;
 
 	if (sock->ssap == 0)
 		return NULL;
 
-	skb = nfc_alloc_send_skb(sock->dev, &sock->sk, MSG_DONTWAIT,
-				 size + LLCP_HEADER_SIZE, &err);
+	headroom = sock->dev->tx_headroom;
+	tailroom = sock->dev->tx_tailroom;
+
+	skb = nfc_alloc_send_skb(&sock->sk, MSG_DONTWAIT,
+				 size + LLCP_HEADER_SIZE, headroom, tailroom,
+				 &err);
 	if (skb == NULL) {
 		pr_err("Could not allocate PDU\n");
 		return NULL;
@@ -734,7 +738,7 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
 	size_t frag_len = 0, remaining_len;
 	u8 *msg_ptr, *msg_data;
 	u16 remote_miu;
-	int err;
+	int err, headroom, tailroom;
 
 	pr_debug("Send UI frame len %zd\n", len);
 
@@ -751,6 +755,9 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
 		return -EFAULT;
 	}
 
+	headroom = sock->dev->tx_headroom;
+	tailroom = sock->dev->tx_tailroom;
+
 	remaining_len = len;
 	msg_ptr = msg_data;
 
@@ -763,8 +770,9 @@ 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,
-					 frag_len + LLCP_HEADER_SIZE, &err);
+		pdu = nfc_alloc_send_skb(&sock->sk, 0,
+					 frag_len + LLCP_HEADER_SIZE,
+					 headroom, tailroom, &err);
 		if (pdu == NULL) {
 			pr_err("Could not allocate PDU (error=%d)\n", err);
 			len -= remaining_len;
diff --git a/net/nfc/rawsock.c b/net/nfc/rawsock.c
index 5125392bb68e..fab1facb7105 100644
--- a/net/nfc/rawsock.c
+++ b/net/nfc/rawsock.c
@@ -207,7 +207,7 @@ static int rawsock_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 	struct sock *sk = sock->sk;
 	struct nfc_dev *dev = nfc_rawsock(sk)->dev;
 	struct sk_buff *skb;
-	int rc;
+	int rc, headroom, tailroom;
 
 	pr_debug("sock=%p sk=%p len=%zu\n", sock, sk, len);
 
@@ -217,7 +217,11 @@ static int rawsock_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 	if (sock->state != SS_CONNECTED)
 		return -ENOTCONN;
 
-	skb = nfc_alloc_send_skb(dev, sk, msg->msg_flags, len, &rc);
+	headroom = dev->tx_headroom;
+	tailroom = dev->tx_tailroom;
+
+	skb = nfc_alloc_send_skb(sk, msg->msg_flags, len, headroom, tailroom,
+				 &rc);
 	if (skb == NULL)
 		return rc;
 
-- 
2.42.0