Skip to content

Commit 73abbaf

Browse files
Muhammad Bilalgregkh
authored andcommitted
Bluetooth: L2CAP: validate option length before reading conf opt value
commit 6876175 upstream. l2cap_get_conf_opt() derives the option length from the attacker-controlled opt->len field and immediately dereferences opt->val (as u8, get_unaligned_le16() or get_unaligned_le32(), or a raw pointer for the default case) before any caller has confirmed that opt->len bytes are present in the buffer. The callers (l2cap_parse_conf_req(), l2cap_parse_conf_rsp() and l2cap_conf_rfc_get()) only detect a malformed option afterwards, once the running length has gone negative, by which point the out-of-bounds read has already executed. An existing post-hoc length check keeps the garbage value from being consumed, so this is not a data leak in the current control flow. It is still a validate-after-use ordering bug: up to 4 bytes are read past the end of the buffer before it is known to contain them, and it is fragile to future changes in the callers. Fix it at the source. Pass the end of the buffer into l2cap_get_conf_opt() and refuse to touch opt->val unless the full option (header + value) fits. Each caller computes an end pointer once before the loop and checks the return value directly instead of inferring the error from a negative length. Fixes: 7c9cbd0 ("Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer") Cc: stable@vger.kernel.org Signed-off-by: Muhammad Bilal <meatuni001@gmail.com> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent d5616be commit 73abbaf

1 file changed

Lines changed: 28 additions & 8 deletions

File tree

net/bluetooth/l2cap_core.c

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3048,13 +3048,24 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn, u8 code,
30483048
return NULL;
30493049
}
30503050

3051-
static inline int l2cap_get_conf_opt(void **ptr, int *type, int *olen,
3052-
unsigned long *val)
3051+
static inline int l2cap_get_conf_opt(void **ptr, void *end, int *type,
3052+
int *olen, unsigned long *val)
30533053
{
30543054
struct l2cap_conf_opt *opt = *ptr;
30553055
int len;
30563056

3057+
/* opt->len is attacker-controlled. Validate that the full option
3058+
* (header + value) actually fits in the buffer before touching
3059+
* opt->val, otherwise the switch below reads past the end of the
3060+
* caller's buffer.
3061+
*/
3062+
if (end - *ptr < L2CAP_CONF_OPT_SIZE)
3063+
return -EINVAL;
3064+
30573065
len = L2CAP_CONF_OPT_SIZE + opt->len;
3066+
if (end - *ptr < len)
3067+
return -EINVAL;
3068+
30583069
*ptr += len;
30593070

30603071
*type = opt->type;
@@ -3426,6 +3437,7 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
34263437
void *ptr = rsp->data;
34273438
void *endptr = data + data_size;
34283439
void *req = chan->conf_req;
3440+
void *req_end = req + chan->conf_len;
34293441
int len = chan->conf_len;
34303442
int type, hint, olen;
34313443
unsigned long val;
@@ -3439,9 +3451,11 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
34393451
BT_DBG("chan %p", chan);
34403452

34413453
while (len >= L2CAP_CONF_OPT_SIZE) {
3442-
len -= l2cap_get_conf_opt(&req, &type, &olen, &val);
3443-
if (len < 0)
3454+
int ret = l2cap_get_conf_opt(&req, req_end, &type, &olen, &val);
3455+
3456+
if (ret < 0)
34443457
break;
3458+
len -= ret;
34453459

34463460
hint = type & L2CAP_CONF_HINT;
34473461
type &= L2CAP_CONF_MASK;
@@ -3669,6 +3683,7 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len,
36693683
struct l2cap_conf_req *req = data;
36703684
void *ptr = req->data;
36713685
void *endptr = data + size;
3686+
void *rsp_end = rsp + len;
36723687
int type, olen;
36733688
unsigned long val;
36743689
struct l2cap_conf_rfc rfc = { .mode = L2CAP_MODE_BASIC };
@@ -3677,9 +3692,11 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len,
36773692
BT_DBG("chan %p, rsp %p, len %d, req %p", chan, rsp, len, data);
36783693

36793694
while (len >= L2CAP_CONF_OPT_SIZE) {
3680-
len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
3681-
if (len < 0)
3695+
int ret = l2cap_get_conf_opt(&rsp, rsp_end, &type, &olen, &val);
3696+
3697+
if (ret < 0)
36823698
break;
3699+
len -= ret;
36833700

36843701
switch (type) {
36853702
case L2CAP_CONF_MTU:
@@ -3930,6 +3947,7 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len)
39303947
{
39313948
int type, olen;
39323949
unsigned long val;
3950+
void *rsp_end = rsp + len;
39333951
/* Use sane default values in case a misbehaving remote device
39343952
* did not send an RFC or extended window size option.
39353953
*/
@@ -3948,9 +3966,11 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len)
39483966
return;
39493967

39503968
while (len >= L2CAP_CONF_OPT_SIZE) {
3951-
len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
3952-
if (len < 0)
3969+
int ret = l2cap_get_conf_opt(&rsp, rsp_end, &type, &olen, &val);
3970+
3971+
if (ret < 0)
39533972
break;
3973+
len -= ret;
39543974

39553975
switch (type) {
39563976
case L2CAP_CONF_RFC:

0 commit comments

Comments
 (0)