Re: [PATCH net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern()
From: Ilia Gavrilov <hidden>
Date: 2025-10-24 08:56:53
Also in:
linux-bluetooth, lkml, stable
On 10/23/25 18:30, Luiz Augusto von Dentz wrote:
Hi Ilia, On Thu, Oct 23, 2025 at 11:08 AM Ilia Gavrilov [off-list ref] wrote:quoted
Hi, Luiz, thank you for the review. On 10/23/25 16:18, Luiz Augusto von Dentz wrote:quoted
Hi Ilia, On Mon, Oct 20, 2025 at 11:12 AM Ilia Gavrilov [off-list ref] wrote:quoted
In the parse_adv_monitor_pattern() function, the value of the 'length' variable is currently limited to HCI_MAX_EXT_AD_LENGTH(251). The size of the 'value' array in the mgmt_adv_pattern structure is 31. If the value of 'pattern[i].length' is set in the user space and exceeds 31, the 'patterns[i].value' array can be accessed out of bound when copied. Increasing the size of the 'value' array in the 'mgmt_adv_pattern' structure will break the userspace. Considering this, and to avoid OOB access revert the limits for 'offset' and 'length' back to the value of HCI_MAX_AD_LENGTH. Found by InfoTeCS on behalf of Linux Verification Center (linuxtesting.org) with SVACE. Fixes: db08722fc7d4 ("Bluetooth: hci_core: Fix missing instances using HCI_MAX_AD_LENGTH") Cc: stable@vger.kernel.org Signed-off-by: Ilia Gavrilov <redacted> --- include/net/bluetooth/mgmt.h | 2 +- net/bluetooth/mgmt.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h index 74edea06985b..4b07ce6dfd69 100644 --- a/include/net/bluetooth/mgmt.h +++ b/include/net/bluetooth/mgmt.h@@ -780,7 +780,7 @@ struct mgmt_adv_pattern { __u8 ad_type; __u8 offset; __u8 length; - __u8 value[31]; + __u8 value[HCI_MAX_AD_LENGTH];Why not use HCI_MAX_EXT_AD_LENGTH above? Or perhaps even make it opaque since the actual size is defined by length - offset.As I see it, user programs rely on this size of the structure, and if the size is changed, they will be broken. Excerpt from bluez tools sources: ... structure of mgmt_adv_pattern { uint8_t ad type; uint8_t offset; length of uint8_t; uint8_t value[31]; } __packed; ...Well it is broken for EA already, so the question is should we leave it to just handle legacy advertisement or not?
From a security point of view, it is better to fix the problem of OOB access of the 'value' array in all kernels starting from version 6.6+. There are two solutions: 1) The simplest solution. Increase the size to the 'value' array in the 'mgmt_adv_pattern' structure, but this type of command will not work: $btmgmt $menu monitor $add-pattern 16:0:01020304050607080900 2) Do as suggested in this patch
At some point I was actually just considering removing/deprecating the support of this command altogether since there exists a standard way to do advertisement monitoring called Monitoring Advertisers introduced in 6.0: https://www.bluetooth.com/core-specification-6-feature-overview/?utm_source=internal&utm_medium=blog&utm_campaign=technical&utm_content=now-available-new-version-of-the-bluetooth-core-specification
It seems to me that it's better to remove/deprecate the support of the command in the next version of the kernel.
The the standard monitoring list doesn't seem to be able to do filtering on the data itself, which I think the where the decision based filtering used, so it is not really compatible with the MS vendor commands.quoted
quoted
quoted
} __packed; #define MGMT_OP_ADD_ADV_PATTERNS_MONITOR 0x0052diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index a3d16eece0d2..500033b70a96 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c@@ -5391,9 +5391,9 @@ static u8 parse_adv_monitor_pattern(struct adv_monitor *m, u8 pattern_count, for (i = 0; i < pattern_count; i++) { offset = patterns[i].offset; length = patterns[i].length; - if (offset >= HCI_MAX_EXT_AD_LENGTH || - length > HCI_MAX_EXT_AD_LENGTH || - (offset + length) > HCI_MAX_EXT_AD_LENGTH) + if (offset >= HCI_MAX_AD_LENGTH || + length > HCI_MAX_AD_LENGTH || + (offset + length) > HCI_MAX_AD_LENGTH) return MGMT_STATUS_INVALID_PARAMS; p = kmalloc(sizeof(*p), GFP_KERNEL); --2.39.5