Module Name: src
Committed By: plunky
Date: Tue Aug 21 14:59:13 UTC 2018
Modified Files:
src/sys/netbt: hci_event.c l2cap_signal.c
Log Message:
Result of audit to check that mbuf length is checked before m_copydata()
and that any data supposedly copied out is valid before use.
prompted by maxv@, I have checked every usage of m_copydata() and made
the following corrections
hci_event.c:
hci_event_command_compl()
check that the packet does contain enough data for there to
be a status code before noting possible failures.
hci_event_num_compl_pkts()
check that the packet does contain data to cover the
stated number of handle/num pairs
l2cap_signal.c:
l2cap_recv_signal()
just ignore packets with not enough data rather than
trying to reject them (may not have cmd.ident)
l2cap_recv_command_rej()
check we have a valid reason and/or data before use
To generate a diff of this commit:
cvs rdiff -u -r1.24 -r1.25 src/sys/netbt/hci_event.c
cvs rdiff -u -r1.18 -r1.19 src/sys/netbt/l2cap_signal.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/netbt/hci_event.c
diff -u src/sys/netbt/hci_event.c:1.24 src/sys/netbt/hci_event.c:1.25
--- src/sys/netbt/hci_event.c:1.24 Sat Nov 28 09:04:34 2015
+++ src/sys/netbt/hci_event.c Tue Aug 21 14:59:13 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: hci_event.c,v 1.24 2015/11/28 09:04:34 plunky Exp $ */
+/* $NetBSD: hci_event.c,v 1.25 2018/08/21 14:59:13 plunky Exp $ */
/*-
* Copyright (c) 2005 Iain Hibbert.
@@ -31,7 +31,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: hci_event.c,v 1.24 2015/11/28 09:04:34 plunky Exp $");
+__KERNEL_RCSID(0, "$NetBSD: hci_event.c,v 1.25 2018/08/21 14:59:13 plunky Exp $");
#include <sys/param.h>
#include <sys/kernel.h>
@@ -316,12 +316,14 @@ hci_event_command_compl(struct hci_unit
* that a command_complete packet will contain the status though most
* do seem to.
*/
- m_copydata(m, 0, sizeof(rp), &rp);
- if (rp.status > 0)
- aprint_error_dev(unit->hci_dev,
- "CommandComplete opcode (%03x|%04x) failed (status=0x%02x)\n",
- HCI_OGF(le16toh(ep.opcode)), HCI_OCF(le16toh(ep.opcode)),
- rp.status);
+ if (m->m_pkthdr.len >= sizeof(rp)) {
+ m_copydata(m, 0, sizeof(rp), &rp);
+ if (rp.status > 0)
+ aprint_error_dev(unit->hci_dev,
+ "CommandComplete opcode (%03x|%04x) failed (status=0x%02x)\n",
+ HCI_OGF(le16toh(ep.opcode)), HCI_OCF(le16toh(ep.opcode)),
+ rp.status);
+ }
/*
* post processing of completed commands
@@ -383,6 +385,9 @@ hci_event_num_compl_pkts(struct hci_unit
m_copydata(m, 0, sizeof(ep), &ep);
m_adj(m, sizeof(ep));
+ if (m->m_pkthdr.len < ep.num_con_handles * (sizeof(handle) + sizeof(num)))
+ return;
+
while (ep.num_con_handles--) {
m_copydata(m, 0, sizeof(handle), &handle);
m_adj(m, sizeof(handle));
Index: src/sys/netbt/l2cap_signal.c
diff -u src/sys/netbt/l2cap_signal.c:1.18 src/sys/netbt/l2cap_signal.c:1.19
--- src/sys/netbt/l2cap_signal.c:1.18 Tue Oct 4 14:13:46 2016
+++ src/sys/netbt/l2cap_signal.c Tue Aug 21 14:59:13 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: l2cap_signal.c,v 1.18 2016/10/04 14:13:46 joerg Exp $ */
+/* $NetBSD: l2cap_signal.c,v 1.19 2018/08/21 14:59:13 plunky Exp $ */
/*-
* Copyright (c) 2005 Iain Hibbert.
@@ -31,7 +31,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: l2cap_signal.c,v 1.18 2016/10/04 14:13:46 joerg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: l2cap_signal.c,v 1.19 2018/08/21 14:59:13 plunky Exp $");
#include <sys/param.h>
#include <sys/kernel.h>
@@ -64,7 +64,8 @@ static void l2cap_qos_htob(void *, l2cap
/*
* process incoming signal packets (CID 0x0001). Can contain multiple
- * requests/responses.
+ * requests/responses. The signal hander should clear the command from
+ * the mbuf before returning.
*/
void
l2cap_recv_signal(struct mbuf *m, struct hci_link *link)
@@ -72,11 +73,8 @@ l2cap_recv_signal(struct mbuf *m, struct
l2cap_cmd_hdr_t cmd;
for(;;) {
- if (m->m_pkthdr.len == 0)
- goto finish;
-
if (m->m_pkthdr.len < sizeof(cmd))
- goto reject;
+ goto finish;
m_copydata(m, 0, sizeof(cmd), &cmd);
cmd.length = le16toh(cmd.length);
@@ -183,32 +181,42 @@ l2cap_recv_command_rej(struct mbuf *m, s
cmd.length = le16toh(cmd.length);
+ /* The length here must contain the reason (2 octets) plus
+ * any data (0 or more octets) but we already know it is not
+ * bigger than l2cap_cmd_rej_cp
+ */
m_copydata(m, 0, cmd.length, &cp);
m_adj(m, cmd.length);
+ if (cmd.length < 2)
+ return;
+
req = l2cap_request_lookup(link, cmd.ident);
if (req == NULL)
return;
switch (le16toh(cp.reason)) {
- case L2CAP_REJ_NOT_UNDERSTOOD:
+ case L2CAP_REJ_NOT_UNDERSTOOD: /* data length = 0 octets */
/*
* I dont know what to do, just move up the timeout
*/
callout_schedule(&req->lr_rtx, 0);
break;
- case L2CAP_REJ_MTU_EXCEEDED:
+ case L2CAP_REJ_MTU_EXCEEDED: /* data length = 2 octets */
/*
* I didnt send any commands over L2CAP_MTU_MINIMUM size, but..
*
* XXX maybe we should resend this, instead?
*/
+ if (cmd.length != 4)
+ return;
+
link->hl_mtu = le16toh(cp.data[0]);
callout_schedule(&req->lr_rtx, 0);
break;
- case L2CAP_REJ_INVALID_CID:
+ case L2CAP_REJ_INVALID_CID: /* data length = 4 octets */
/*
* Well, if they dont have such a channel then our channel is
* most likely closed. Make it so.
