[PATCH] Bluetooth: RFCOMM: validate skb length in MCC handlers

SeungJu Cheon <[email protected]>
Newsgroups dev.linux.lists.linux-kernel-mentees,org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
rfcomm_recv_pn(), rfcomm_recv_rpn(), rfcomm_recv_rls(), and
rfcomm_recv_msc() cast skb->data to their respective structs
without first checking skb->len. A remote device can send a
short MCC frame, causing out-of-bounds reads from the skb buffer.

For rfcomm_recv_pn(), the uninitialized pn->mtu value is stored
in d->mtu via rfcomm_apply_pn(), then echoed back to the remote
device in the PN response, leaking kernel heap data.

This results in use of uninitialized memory, as reported by KMSAN.

Add explicit skb->len checks against the expected structure size
at the start of each handler before accessing the payload.

=====================================================
BUG: KMSAN: uninit-value in rfcomm_run+0x7eae/0xee90
 rfcomm_run+0x7eae/0xee90
 kthread+0x53f/0x600
 ret_from_fork+0x20f/0x910
 ret_from_fork_asm+0x1a/0x30

Uninit was created at:
 kmem_cache_alloc_node_noprof+0x3cd/0x12d0
 __alloc_skb+0x855/0x1190
 vhci_write+0x125/0x960
 vfs_write+0xbe1/0x15c0
 ksys_write+0x1d9/0x470
 __x64_sys_write+0x97/0xf0
 x64_sys_call+0x2ff0/0x3ea0
 do_syscall_64+0x134/0xf80
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

CPU: 0 UID: 0 PID: 3374 Comm: krfcommd Tainted: G        W           7.0.0-rc7
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
Kernel panic - not syncing: kmsan.panic set ...
=====================================================

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: SeungJu Cheon <[email protected]>
---
 net/bluetooth/rfcomm/core.c | 40 +++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 611a9a94151e..daeba71a1514 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1431,9 +1431,15 @@ static int rfcomm_apply_pn(struct rfcomm_dlc *d, int cr, struct rfcomm_pn *pn)
 
 static int rfcomm_recv_pn(struct rfcomm_session *s, int cr, struct sk_buff *skb)
 {
-	struct rfcomm_pn *pn = (void *) skb->data;
+	struct rfcomm_pn *pn;
 	struct rfcomm_dlc *d;
-	u8 dlci = pn->dlci;
+	u8 dlci;
+
+	if (skb->len < sizeof(*pn))
+		return -EINVAL;
+
+	pn = (void *) skb->data;
+	dlci = pn->dlci;
 
 	BT_DBG("session %p state %ld dlci %d", s, s->state, dlci);
 
@@ -1483,8 +1489,8 @@ static int rfcomm_recv_pn(struct rfcomm_session *s, int cr, struct sk_buff *skb)
 
 static int rfcomm_recv_rpn(struct rfcomm_session *s, int cr, int len, struct sk_buff *skb)
 {
-	struct rfcomm_rpn *rpn = (void *) skb->data;
-	u8 dlci = __get_dlci(rpn->dlci);
+	struct rfcomm_rpn *rpn;
+	u8 dlci;
 
 	u8 bit_rate  = 0;
 	u8 data_bits = 0;
@@ -1495,6 +1501,12 @@ static int rfcomm_recv_rpn(struct rfcomm_session *s, int cr, int len, struct sk_
 	u8 xoff_char = 0;
 	u16 rpn_mask = RFCOMM_RPN_PM_ALL;
 
+	if (skb->len < sizeof(*rpn))
+		return -EINVAL;
+
+	rpn = (void *) skb->data;
+	dlci = __get_dlci(rpn->dlci);
+
 	BT_DBG("dlci %d cr %d len 0x%x bitr 0x%x line 0x%x flow 0x%x xonc 0x%x xoffc 0x%x pm 0x%x",
 		dlci, cr, len, rpn->bit_rate, rpn->line_settings, rpn->flow_ctrl,
 		rpn->xon_char, rpn->xoff_char, rpn->param_mask);
@@ -1589,8 +1601,14 @@ static int rfcomm_recv_rpn(struct rfcomm_session *s, int cr, int len, struct sk_
 
 static int rfcomm_recv_rls(struct rfcomm_session *s, int cr, struct sk_buff *skb)
 {
-	struct rfcomm_rls *rls = (void *) skb->data;
-	u8 dlci = __get_dlci(rls->dlci);
+	struct rfcomm_rls *rls;
+	u8 dlci;
+
+	if (skb->len < sizeof(*rls))
+		return -EINVAL;
+
+	rls = (void *) skb->data;
+	dlci = __get_dlci(rls->dlci);
 
 	BT_DBG("dlci %d cr %d status 0x%x", dlci, cr, rls->status);
 
@@ -1608,9 +1626,15 @@ static int rfcomm_recv_rls(struct rfcomm_session *s, int cr, struct sk_buff *skb
 
 static int rfcomm_recv_msc(struct rfcomm_session *s, int cr, struct sk_buff *skb)
 {
-	struct rfcomm_msc *msc = (void *) skb->data;
+	struct rfcomm_msc *msc;
 	struct rfcomm_dlc *d;
-	u8 dlci = __get_dlci(msc->dlci);
+	u8 dlci;
+
+	if (skb->len < sizeof(*msc))
+		return -EINVAL;
+
+	msc = (void *) skb->data;
+	dlci = __get_dlci(msc->dlci);
 
 	BT_DBG("dlci %d cr %d v24 0x%x", dlci, cr, msc->v24_sig);
 
-- 
2.52.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.