diff options
| author | plunky <plunky@NetBSD.org> | 2018-08-21 14:59:13 +0000 |
|---|---|---|
| committer | plunky <plunky@NetBSD.org> | 2018-08-21 14:59:13 +0000 |
| commit | 8ff2796d636d64da93d7a9abfaa88c504617ff6d (patch) | |
| tree | 37b7d6658e8bfc2c7c6d5f079f841cb07fe34b8e /sys/netbt | |
| parent | 971031650a5bb1d3e065832068e5dcefd1f12c8c (diff) | |
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
Diffstat (limited to 'sys/netbt')
| -rw-r--r-- | sys/netbt/hci_event.c | 21 | ||||
| -rw-r--r-- | sys/netbt/l2cap_signal.c | 28 |
2 files changed, 31 insertions, 18 deletions
diff --git a/sys/netbt/hci_event.c b/sys/netbt/hci_event.c index fffe0dbcfb5..1296fac2ebe 100644 --- a/sys/netbt/hci_event.c +++ b/sys/netbt/hci_event.c @@ -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 *unit, struct mbuf *m) * 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 *unit, struct mbuf *m) 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)); diff --git a/sys/netbt/l2cap_signal.c b/sys/netbt/l2cap_signal.c index 4c628b230cc..6e86907d158 100644 --- a/sys/netbt/l2cap_signal.c +++ b/sys/netbt/l2cap_signal.c @@ -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_qos_t *); /* * 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 hci_link *link) 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, struct hci_link *link) 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. |
