bt: host/classic: Fix possible integer overflow #97370
Conversation
zephyrbot
added
area: Bluetooth
area: Bluetooth Classic
zephyrbot
requested review from
MarkWangChinese,
chengkai15,
gzh-terry,
jhedberg,
lylezhu2012 and
makeshi
|
build failure is not related with the change. |
| if (sys_le16_to_cpu(hdr->len) > (UINT16_MAX - sizeof(*hdr))) { | ||
| LOG_ERR("L2CAP PDU length overflow"); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Wouldn't it be simpler to just change acl_total_len to be uint32_t? That'll likely generate a bit more efficient code to since it's the native word size on most Zephyr platforms.
There was a problem hiding this comment.
Yep, that works. I hadn't seen hdr->len type.
There was a problem hiding this comment.
Normally we have the host endianness type sizes match the protocol type size, but in this case using a larger host endian type seems like a reasonable option to avoid pitfalls with the subsequent arithmetic.
There was a problem hiding this comment.
My only concern with changing the type here is that if that structure changes its type hdr->len we will silently end up with the same problem again.
There was a problem hiding this comment.
Which structure? The type of hdr? That's based on a standard protocol specification, so it will never change.
Invalid header length and cause an integer overflow in bt_br_acl_recv leading to undesired behavior. Signed-off-by: Flavio Ceolin <[email protected]>
|
5 participants
Invalid header length and cause an integer overflow in bt_br_acl_recv leading to undesired behavior.