public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern()
@ 2025-10-20 15:12 Ilia Gavrilov
  2025-10-20 15:36 ` [net] " bluez.test.bot
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ilia Gavrilov @ 2025-10-20 15:12 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Johan Hedberg, Luiz Augusto von Dentz, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, lvc-project@linuxtesting.org,
	stable@vger.kernel.org

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];
 } __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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* RE: [net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern()
  2025-10-20 15:12 [PATCH net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern() Ilia Gavrilov
@ 2025-10-20 15:36 ` bluez.test.bot
  2025-10-23 13:18 ` [PATCH net] " Luiz Augusto von Dentz
  2025-10-31 15:30 ` patchwork-bot+bluetooth
  2 siblings, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2025-10-20 15:36 UTC (permalink / raw)
  To: linux-bluetooth, Ilia.Gavrilov

[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1013638

---Test result---

Test Summary:
CheckPatch                    PENDING   0.45 seconds
GitLint                       PENDING   0.30 seconds
SubjectPrefix                 PASS      0.08 seconds
BuildKernel                   PASS      24.53 seconds
CheckAllWarning               PASS      27.03 seconds
CheckSparse                   PASS      30.34 seconds
BuildKernel32                 PASS      24.41 seconds
TestRunnerSetup               PASS      488.14 seconds
TestRunner_l2cap-tester       PASS      23.93 seconds
TestRunner_iso-tester         PASS      93.03 seconds
TestRunner_bnep-tester        PASS      6.17 seconds
TestRunner_mgmt-tester        FAIL      112.90 seconds
TestRunner_rfcomm-tester      PASS      9.38 seconds
TestRunner_sco-tester         PASS      14.47 seconds
TestRunner_ioctl-tester       FAIL      10.38 seconds
TestRunner_mesh-tester        FAIL      12.31 seconds
TestRunner_smp-tester         PASS      8.52 seconds
TestRunner_userchan-tester    PASS      6.40 seconds
IncrementalBuild              PENDING   0.46 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 485 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.108 seconds
##############################
Test: TestRunner_ioctl-tester - FAIL
Desc: Run ioctl-tester with test-runner
Output:
Total: 28, Passed: 26 (92.9%), Failed: 2, Not Run: 0

Failed Test Cases
Connection List                                      Failed       1.059 seconds
Connection Info                                      Failed       0.133 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.764 seconds
Mesh - Send cancel - 2                               Timed out    1.999 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern()
  2025-10-20 15:12 [PATCH net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern() Ilia Gavrilov
  2025-10-20 15:36 ` [net] " bluez.test.bot
@ 2025-10-23 13:18 ` Luiz Augusto von Dentz
  2025-10-23 15:08   ` Ilia Gavrilov
  2025-10-31 15:30 ` patchwork-bot+bluetooth
  2 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2025-10-23 13:18 UTC (permalink / raw)
  To: Ilia Gavrilov
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman,
	linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, lvc-project@linuxtesting.org,
	stable@vger.kernel.org

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.

>  } __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



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern()
  2025-10-23 13:18 ` [PATCH net] " Luiz Augusto von Dentz
@ 2025-10-23 15:08   ` Ilia Gavrilov
  2025-10-23 15:30     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 7+ messages in thread
From: Ilia Gavrilov @ 2025-10-23 15:08 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman,
	linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, lvc-project@linuxtesting.org,
	stable@vger.kernel.org

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;
...


>>  } __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
> 
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern()
  2025-10-23 15:08   ` Ilia Gavrilov
@ 2025-10-23 15:30     ` Luiz Augusto von Dentz
  2025-10-24  8:56       ` Ilia Gavrilov
  0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2025-10-23 15:30 UTC (permalink / raw)
  To: Ilia Gavrilov
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman,
	linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, lvc-project@linuxtesting.org,
	stable@vger.kernel.org

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? 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

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
> >
> >
> >
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern()
  2025-10-23 15:30     ` Luiz Augusto von Dentz
@ 2025-10-24  8:56       ` Ilia Gavrilov
  0 siblings, 0 replies; 7+ messages in thread
From: Ilia Gavrilov @ 2025-10-24  8:56 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman,
	linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, lvc-project@linuxtesting.org,
	stable@vger.kernel.org

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=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.
> 
>>
>>>>  } __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
>>>
>>>
>>>
>>
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern()
  2025-10-20 15:12 [PATCH net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern() Ilia Gavrilov
  2025-10-20 15:36 ` [net] " bluez.test.bot
  2025-10-23 13:18 ` [PATCH net] " Luiz Augusto von Dentz
@ 2025-10-31 15:30 ` patchwork-bot+bluetooth
  2 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+bluetooth @ 2025-10-31 15:30 UTC (permalink / raw)
  To: Ilia Gavrilov
  Cc: marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba, pabeni,
	horms, linux-bluetooth, netdev, linux-kernel, lvc-project, stable

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Mon, 20 Oct 2025 15:12:55 +0000 you 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.
> 
> [...]

Here is the summary with links:
  - [net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern()
    https://git.kernel.org/bluetooth/bluetooth-next/c/e1e9d861e2f9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-10-31 15:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-20 15:12 [PATCH net] Bluetooth: MGMT: Fix OOB access in parse_adv_monitor_pattern() Ilia Gavrilov
2025-10-20 15:36 ` [net] " bluez.test.bot
2025-10-23 13:18 ` [PATCH net] " Luiz Augusto von Dentz
2025-10-23 15:08   ` Ilia Gavrilov
2025-10-23 15:30     ` Luiz Augusto von Dentz
2025-10-24  8:56       ` Ilia Gavrilov
2025-10-31 15:30 ` patchwork-bot+bluetooth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox