| From af3d5d1c87664a4f150fcf3534c6567cb19909b0 Mon Sep 17 00:00:00 2001 |
| From: Marcel Holtmann <marcel@holtmann.org> |
| Date: Fri, 18 Jan 2019 12:56:20 +0100 |
| Subject: Bluetooth: Check L2CAP option sizes returned from l2cap_get_conf_opt |
| |
| From: Marcel Holtmann <marcel@holtmann.org> |
| |
| commit af3d5d1c87664a4f150fcf3534c6567cb19909b0 upstream. |
| |
| When doing option parsing for standard type values of 1, 2 or 4 octets, |
| the value is converted directly into a variable instead of a pointer. To |
| avoid being tricked into being a pointer, check that for these option |
| types that sizes actually match. In L2CAP every option is fixed size and |
| thus it is prudent anyway to ensure that the remote side sends us the |
| right option size along with option paramters. |
| |
| If the option size is not matching the option type, then that option is |
| silently ignored. It is a protocol violation and instead of trying to |
| give the remote attacker any further hints just pretend that option is |
| not present and proceed with the default values. Implementation |
| following the specification and its qualification procedures will always |
| use the correct size and thus not being impacted here. |
| |
| To keep the code readable and consistent accross all options, a few |
| cosmetic changes were also required. |
| |
| Signed-off-by: Marcel Holtmann <marcel@holtmann.org> |
| Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| Signed-off-by: Johan Hedberg <johan.hedberg@intel.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| net/bluetooth/l2cap_core.c | 77 ++++++++++++++++++++++++++------------------- |
| 1 file changed, 46 insertions(+), 31 deletions(-) |
| |
| --- a/net/bluetooth/l2cap_core.c |
| +++ b/net/bluetooth/l2cap_core.c |
| @@ -3342,10 +3342,14 @@ static int l2cap_parse_conf_req(struct l |
| |
| switch (type) { |
| case L2CAP_CONF_MTU: |
| + if (olen != 2) |
| + break; |
| mtu = val; |
| break; |
| |
| case L2CAP_CONF_FLUSH_TO: |
| + if (olen != 2) |
| + break; |
| chan->flush_to = val; |
| break; |
| |
| @@ -3353,26 +3357,30 @@ static int l2cap_parse_conf_req(struct l |
| break; |
| |
| case L2CAP_CONF_RFC: |
| - if (olen == sizeof(rfc)) |
| - memcpy(&rfc, (void *) val, olen); |
| + if (olen != sizeof(rfc)) |
| + break; |
| + memcpy(&rfc, (void *) val, olen); |
| break; |
| |
| case L2CAP_CONF_FCS: |
| + if (olen != 1) |
| + break; |
| if (val == L2CAP_FCS_NONE) |
| set_bit(CONF_RECV_NO_FCS, &chan->conf_state); |
| break; |
| |
| case L2CAP_CONF_EFS: |
| - if (olen == sizeof(efs)) { |
| - remote_efs = 1; |
| - memcpy(&efs, (void *) val, olen); |
| - } |
| + if (olen != sizeof(efs)) |
| + break; |
| + remote_efs = 1; |
| + memcpy(&efs, (void *) val, olen); |
| break; |
| |
| case L2CAP_CONF_EWS: |
| + if (olen != 2) |
| + break; |
| if (!(chan->conn->local_fixed_chan & L2CAP_FC_A2MP)) |
| return -ECONNREFUSED; |
| - |
| set_bit(FLAG_EXT_CTRL, &chan->flags); |
| set_bit(CONF_EWS_RECV, &chan->conf_state); |
| chan->tx_win_max = L2CAP_DEFAULT_EXT_WINDOW; |
| @@ -3382,7 +3390,6 @@ static int l2cap_parse_conf_req(struct l |
| default: |
| if (hint) |
| break; |
| - |
| result = L2CAP_CONF_UNKNOWN; |
| *((u8 *) ptr++) = type; |
| break; |
| @@ -3550,55 +3557,60 @@ static int l2cap_parse_conf_rsp(struct l |
| |
| switch (type) { |
| case L2CAP_CONF_MTU: |
| + if (olen != 2) |
| + break; |
| if (val < L2CAP_DEFAULT_MIN_MTU) { |
| *result = L2CAP_CONF_UNACCEPT; |
| chan->imtu = L2CAP_DEFAULT_MIN_MTU; |
| } else |
| chan->imtu = val; |
| - l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, endptr - ptr); |
| + l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, chan->imtu, |
| + endptr - ptr); |
| break; |
| |
| case L2CAP_CONF_FLUSH_TO: |
| + if (olen != 2) |
| + break; |
| chan->flush_to = val; |
| - l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, |
| - 2, chan->flush_to, endptr - ptr); |
| + l2cap_add_conf_opt(&ptr, L2CAP_CONF_FLUSH_TO, 2, |
| + chan->flush_to, endptr - ptr); |
| break; |
| |
| case L2CAP_CONF_RFC: |
| - if (olen == sizeof(rfc)) |
| - memcpy(&rfc, (void *)val, olen); |
| - |
| + if (olen != sizeof(rfc)) |
| + break; |
| + memcpy(&rfc, (void *)val, olen); |
| if (test_bit(CONF_STATE2_DEVICE, &chan->conf_state) && |
| rfc.mode != chan->mode) |
| return -ECONNREFUSED; |
| - |
| chan->fcs = 0; |
| - |
| - l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, |
| - sizeof(rfc), (unsigned long) &rfc, endptr - ptr); |
| + l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc), |
| + (unsigned long) &rfc, endptr - ptr); |
| break; |
| |
| case L2CAP_CONF_EWS: |
| + if (olen != 2) |
| + break; |
| chan->ack_win = min_t(u16, val, chan->ack_win); |
| l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS, 2, |
| chan->tx_win, endptr - ptr); |
| break; |
| |
| case L2CAP_CONF_EFS: |
| - if (olen == sizeof(efs)) { |
| - memcpy(&efs, (void *)val, olen); |
| - |
| - if (chan->local_stype != L2CAP_SERV_NOTRAFIC && |
| - efs.stype != L2CAP_SERV_NOTRAFIC && |
| - efs.stype != chan->local_stype) |
| - return -ECONNREFUSED; |
| - |
| - l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), |
| - (unsigned long) &efs, endptr - ptr); |
| - } |
| + if (olen != sizeof(efs)) |
| + break; |
| + memcpy(&efs, (void *)val, olen); |
| + if (chan->local_stype != L2CAP_SERV_NOTRAFIC && |
| + efs.stype != L2CAP_SERV_NOTRAFIC && |
| + efs.stype != chan->local_stype) |
| + return -ECONNREFUSED; |
| + l2cap_add_conf_opt(&ptr, L2CAP_CONF_EFS, sizeof(efs), |
| + (unsigned long) &efs, endptr - ptr); |
| break; |
| |
| case L2CAP_CONF_FCS: |
| + if (olen != 1) |
| + break; |
| if (*result == L2CAP_CONF_PENDING) |
| if (val == L2CAP_FCS_NONE) |
| set_bit(CONF_RECV_NO_FCS, |
| @@ -3730,10 +3742,13 @@ static void l2cap_conf_rfc_get(struct l2 |
| |
| switch (type) { |
| case L2CAP_CONF_RFC: |
| - if (olen == sizeof(rfc)) |
| - memcpy(&rfc, (void *)val, olen); |
| + if (olen != sizeof(rfc)) |
| + break; |
| + memcpy(&rfc, (void *)val, olen); |
| break; |
| case L2CAP_CONF_EWS: |
| + if (olen != 2) |
| + break; |
| txwin_ext = val; |
| break; |
| } |