Aren
/
NetBSD
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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
This commit is contained in:
plunky 2018-08-21 14:59:13 +00:00
parent 83927ccc37
commit 1834dc7936
2 changed files with 31 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
sys/netbt/hci_event.c
View File

@ -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. * Copyright (c) 2005 Iain Hibbert.
@ -31,7 +31,7 @@
*/ */
#include <sys/cdefs.h> #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/param.h>
#include <sys/kernel.h> #include <sys/kernel.h>
@ -316,12 +316,14 @@ hci_event_command_compl(struct hci_unit *unit, struct mbuf *m)
* that a command_complete packet will contain the status though most * that a command_complete packet will contain the status though most
* do seem to. * do seem to.
*/ */
m_copydata(m, 0, sizeof(rp), &rp); if (m->m_pkthdr.len >= sizeof(rp)) {
if (rp.status > 0) m_copydata(m, 0, sizeof(rp), &rp);
aprint_error_dev(unit->hci_dev, if (rp.status > 0)
"CommandComplete opcode (%03x|%04x) failed (status=0x%02x)\n", aprint_error_dev(unit->hci_dev,
HCI_OGF(le16toh(ep.opcode)), HCI_OCF(le16toh(ep.opcode)), "CommandComplete opcode (%03x|%04x) failed (status=0x%02x)\n",
rp.status); HCI_OGF(le16toh(ep.opcode)), HCI_OCF(le16toh(ep.opcode)),
rp.status);
}
/* /*
* post processing of completed commands * post processing of completed commands
@ -383,6 +385,9 @@ hci_event_num_compl_pkts(struct hci_unit *unit, struct mbuf *m)
m_copydata(m, 0, sizeof(ep), &ep); m_copydata(m, 0, sizeof(ep), &ep);
m_adj(m, sizeof(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--) { while (ep.num_con_handles--) {
m_copydata(m, 0, sizeof(handle), &handle); m_copydata(m, 0, sizeof(handle), &handle);
m_adj(m, sizeof(handle)); m_adj(m, sizeof(handle));

28
sys/netbt/l2cap_signal.c
View File

@ -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. * Copyright (c) 2005 Iain Hibbert.
@ -31,7 +31,7 @@
*/ */
#include <sys/cdefs.h> #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/param.h>
#include <sys/kernel.h> #include <sys/kernel.h>
@ -64,7 +64,8 @@ static void l2cap_qos_htob(void *, l2cap_qos_t *);
/* /*
* process incoming signal packets (CID 0x0001). Can contain multiple * 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 void
l2cap_recv_signal(struct mbuf *m, struct hci_link *link) l2cap_recv_signal(struct mbuf *m, struct hci_link *link)
@ -72,11 +73,8 @@ l2cap_recv_signal(struct mbuf *m, struct hci_link *link)
l2cap_cmd_hdr_t cmd; l2cap_cmd_hdr_t cmd;
for(;;) { for(;;) {
if (m->m_pkthdr.len == 0)
goto finish;
if (m->m_pkthdr.len < sizeof(cmd)) if (m->m_pkthdr.len < sizeof(cmd))
goto reject; goto finish;
m_copydata(m, 0, sizeof(cmd), &cmd); m_copydata(m, 0, sizeof(cmd), &cmd);
cmd.length = le16toh(cmd.length); cmd.length = le16toh(cmd.length);
@ -183,32 +181,42 @@ l2cap_recv_command_rej(struct mbuf *m, struct hci_link *link)
cmd.length = le16toh(cmd.length); 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_copydata(m, 0, cmd.length, &cp);
m_adj(m, cmd.length); m_adj(m, cmd.length);
if (cmd.length < 2)
return;
req = l2cap_request_lookup(link, cmd.ident); req = l2cap_request_lookup(link, cmd.ident);
if (req == NULL) if (req == NULL)
return; return;
switch (le16toh(cp.reason)) { 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 * I dont know what to do, just move up the timeout
*/ */
callout_schedule(&req->lr_rtx, 0); callout_schedule(&req->lr_rtx, 0);
break; 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.. * I didnt send any commands over L2CAP_MTU_MINIMUM size, but..
* *
* XXX maybe we should resend this, instead? * XXX maybe we should resend this, instead?
*/ */
if (cmd.length != 4)
return;
link->hl_mtu = le16toh(cp.data[0]); link->hl_mtu = le16toh(cp.data[0]);
callout_schedule(&req->lr_rtx, 0); callout_schedule(&req->lr_rtx, 0);
break; 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 * Well, if they dont have such a channel then our channel is
* most likely closed. Make it so. * most likely closed. Make it so.