On 10/23/25 18:30, Luiz Augusto von Dentz wrote:
Hi Ilia,
On Thu, Oct 23, 2025 at 11:08 AM Ilia Gavrilov Ilia.Gavrilov@infotecs.ru wrote:
Hi, Luiz, thank you for the review.
On 10/23/25 16:18, Luiz Augusto von Dentz wrote:
Hi Ilia,
On Mon, Oct 20, 2025 at 11:12 AM Ilia Gavrilov Ilia.Gavrilov@infotecs.ru wrote:
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 Ilia.Gavrilov@infotecs.ru
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=...
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.
} __packed;
#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR 0x0052 diff --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