* [PATCH] Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer
@ 2019-01-18 12:43 Marcel Holtmann
2019-01-18 12:53 ` Greg KH
2019-01-23 11:37 ` Johan Hedberg
0 siblings, 2 replies; 8+ messages in thread
From: Marcel Holtmann @ 2019-01-18 12:43 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gregkh
The function l2cap_get_conf_opt will return L2CAP_CONF_OPT_SIZE + opt->len
as length value. The opt->len however is in control over the remote user
and can be used by an attacker to gain access beyond the bounds of the
actual packet.
To prevent any potential leak of heap memory, it is enough to check that
the resulting len calculation after calling l2cap_get_conf_opt is not
below zero. A well formed packet will always return >= 0 here and will
end with the length value being zero after the last option has been
parsed. In case of malformed packets messing with the opt->len field the
length value will become negative. If that is the case, then just abort
and ignore the option.
In case an attacker uses a too short opt->len value, then garbage will
be parsed, but that is protected by the unknown option handling and also
the option parameter size checks.
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
net/bluetooth/l2cap_core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 77799e7d5a34..ccdc5c67d22a 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3337,6 +3337,8 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
while (len >= L2CAP_CONF_OPT_SIZE) {
len -= l2cap_get_conf_opt(&req, &type, &olen, &val);
+ if (len < 0)
+ break;
hint = type & L2CAP_CONF_HINT;
type &= L2CAP_CONF_MASK;
@@ -3555,6 +3557,8 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len,
while (len >= L2CAP_CONF_OPT_SIZE) {
len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
+ if (len < 0)
+ break;
switch (type) {
case L2CAP_CONF_MTU:
@@ -3740,6 +3744,8 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len)
while (len >= L2CAP_CONF_OPT_SIZE) {
len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
+ if (len < 0)
+ break;
switch (type) {
case L2CAP_CONF_RFC:
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer
2019-01-18 12:43 [PATCH] Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer Marcel Holtmann
@ 2019-01-18 12:53 ` Greg KH
2019-01-18 13:12 ` Marcel Holtmann
2019-01-23 11:37 ` Johan Hedberg
1 sibling, 1 reply; 8+ messages in thread
From: Greg KH @ 2019-01-18 12:53 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
On Fri, Jan 18, 2019 at 01:43:19PM +0100, Marcel Holtmann wrote:
> The function l2cap_get_conf_opt will return L2CAP_CONF_OPT_SIZE + opt->len
> as length value. The opt->len however is in control over the remote user
> and can be used by an attacker to gain access beyond the bounds of the
> actual packet.
>
> To prevent any potential leak of heap memory, it is enough to check that
> the resulting len calculation after calling l2cap_get_conf_opt is not
> below zero. A well formed packet will always return >= 0 here and will
> end with the length value being zero after the last option has been
> parsed. In case of malformed packets messing with the opt->len field the
> length value will become negative. If that is the case, then just abort
> and ignore the option.
>
> In case an attacker uses a too short opt->len value, then garbage will
> be parsed, but that is protected by the unknown option handling and also
> the option parameter size checks.
>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
> net/bluetooth/l2cap_core.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 77799e7d5a34..ccdc5c67d22a 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3337,6 +3337,8 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
>
> while (len >= L2CAP_CONF_OPT_SIZE) {
> len -= l2cap_get_conf_opt(&req, &type, &olen, &val);
> + if (len < 0)
> + break;
<snip>
Patch looks good to me, thanks for fixing this all up:
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer
2019-01-18 12:53 ` Greg KH
@ 2019-01-18 13:12 ` Marcel Holtmann
2019-01-18 15:00 ` Greg KH
0 siblings, 1 reply; 8+ messages in thread
From: Marcel Holtmann @ 2019-01-18 13:12 UTC (permalink / raw)
To: Greg KH; +Cc: linux-bluetooth
Hi Greg,
>> The function l2cap_get_conf_opt will return L2CAP_CONF_OPT_SIZE + opt->len
>> as length value. The opt->len however is in control over the remote user
>> and can be used by an attacker to gain access beyond the bounds of the
>> actual packet.
>>
>> To prevent any potential leak of heap memory, it is enough to check that
>> the resulting len calculation after calling l2cap_get_conf_opt is not
>> below zero. A well formed packet will always return >= 0 here and will
>> end with the length value being zero after the last option has been
>> parsed. In case of malformed packets messing with the opt->len field the
>> length value will become negative. If that is the case, then just abort
>> and ignore the option.
>>
>> In case an attacker uses a too short opt->len value, then garbage will
>> be parsed, but that is protected by the unknown option handling and also
>> the option parameter size checks.
>>
>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
>> ---
>> net/bluetooth/l2cap_core.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 77799e7d5a34..ccdc5c67d22a 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -3337,6 +3337,8 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
>>
>> while (len >= L2CAP_CONF_OPT_SIZE) {
>> len -= l2cap_get_conf_opt(&req, &type, &olen, &val);
>> + if (len < 0)
>> + break;
>
> <snip>
>
> Patch looks good to me, thanks for fixing this all up:
it would be still good if we can get this verified by the reporter.
Regards
Marcel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer
2019-01-18 13:12 ` Marcel Holtmann
@ 2019-01-18 15:00 ` Greg KH
2019-01-21 14:51 ` Marcel Holtmann
0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2019-01-18 15:00 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
On Fri, Jan 18, 2019 at 02:12:23PM +0100, Marcel Holtmann wrote:
> Hi Greg,
>
> >> The function l2cap_get_conf_opt will return L2CAP_CONF_OPT_SIZE + opt->len
> >> as length value. The opt->len however is in control over the remote user
> >> and can be used by an attacker to gain access beyond the bounds of the
> >> actual packet.
> >>
> >> To prevent any potential leak of heap memory, it is enough to check that
> >> the resulting len calculation after calling l2cap_get_conf_opt is not
> >> below zero. A well formed packet will always return >= 0 here and will
> >> end with the length value being zero after the last option has been
> >> parsed. In case of malformed packets messing with the opt->len field the
> >> length value will become negative. If that is the case, then just abort
> >> and ignore the option.
> >>
> >> In case an attacker uses a too short opt->len value, then garbage will
> >> be parsed, but that is protected by the unknown option handling and also
> >> the option parameter size checks.
> >>
> >> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> >> ---
> >> net/bluetooth/l2cap_core.c | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >> index 77799e7d5a34..ccdc5c67d22a 100644
> >> --- a/net/bluetooth/l2cap_core.c
> >> +++ b/net/bluetooth/l2cap_core.c
> >> @@ -3337,6 +3337,8 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
> >>
> >> while (len >= L2CAP_CONF_OPT_SIZE) {
> >> len -= l2cap_get_conf_opt(&req, &type, &olen, &val);
> >> + if (len < 0)
> >> + break;
> >
> > <snip>
> >
> > Patch looks good to me, thanks for fixing this all up:
>
> it would be still good if we can get this verified by the reporter.
The "reporter" seems to have disappeared once they reported this stuff,
so I would not count on them doing anything here, we asked numerous
times :(
If the patches look correct, I recommend just merging it and I can
backport it to the stable releases and the distros can pick it up from
there.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer
2019-01-18 15:00 ` Greg KH
@ 2019-01-21 14:51 ` Marcel Holtmann
2019-01-21 15:09 ` Greg KH
0 siblings, 1 reply; 8+ messages in thread
From: Marcel Holtmann @ 2019-01-21 14:51 UTC (permalink / raw)
To: Greg KH; +Cc: linux-bluetooth
Hi Greg,
>>>> The function l2cap_get_conf_opt will return L2CAP_CONF_OPT_SIZE + opt->len
>>>> as length value. The opt->len however is in control over the remote user
>>>> and can be used by an attacker to gain access beyond the bounds of the
>>>> actual packet.
>>>>
>>>> To prevent any potential leak of heap memory, it is enough to check that
>>>> the resulting len calculation after calling l2cap_get_conf_opt is not
>>>> below zero. A well formed packet will always return >= 0 here and will
>>>> end with the length value being zero after the last option has been
>>>> parsed. In case of malformed packets messing with the opt->len field the
>>>> length value will become negative. If that is the case, then just abort
>>>> and ignore the option.
>>>>
>>>> In case an attacker uses a too short opt->len value, then garbage will
>>>> be parsed, but that is protected by the unknown option handling and also
>>>> the option parameter size checks.
>>>>
>>>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
>>>> ---
>>>> net/bluetooth/l2cap_core.c | 6 ++++++
>>>> 1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>>>> index 77799e7d5a34..ccdc5c67d22a 100644
>>>> --- a/net/bluetooth/l2cap_core.c
>>>> +++ b/net/bluetooth/l2cap_core.c
>>>> @@ -3337,6 +3337,8 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
>>>>
>>>> while (len >= L2CAP_CONF_OPT_SIZE) {
>>>> len -= l2cap_get_conf_opt(&req, &type, &olen, &val);
>>>> + if (len < 0)
>>>> + break;
>>>
>>> <snip>
>>>
>>> Patch looks good to me, thanks for fixing this all up:
>>
>> it would be still good if we can get this verified by the reporter.
>
> The "reporter" seems to have disappeared once they reported this stuff,
> so I would not count on them doing anything here, we asked numerous
> times :(
>
> If the patches look correct, I recommend just merging it and I can
> backport it to the stable releases and the distros can pick it up from
> there.
I think that I have Johan merge them into bluetooth-next first and let them sit a little so we can verify they do not break anything else. You can pick them up later into -stable if nobody complained and we didn’t break qualification.
Regards
Marcel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer
2019-01-21 14:51 ` Marcel Holtmann
@ 2019-01-21 15:09 ` Greg KH
0 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2019-01-21 15:09 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
On Mon, Jan 21, 2019 at 03:51:11PM +0100, Marcel Holtmann wrote:
> Hi Greg,
>
> >>>> The function l2cap_get_conf_opt will return L2CAP_CONF_OPT_SIZE + opt->len
> >>>> as length value. The opt->len however is in control over the remote user
> >>>> and can be used by an attacker to gain access beyond the bounds of the
> >>>> actual packet.
> >>>>
> >>>> To prevent any potential leak of heap memory, it is enough to check that
> >>>> the resulting len calculation after calling l2cap_get_conf_opt is not
> >>>> below zero. A well formed packet will always return >= 0 here and will
> >>>> end with the length value being zero after the last option has been
> >>>> parsed. In case of malformed packets messing with the opt->len field the
> >>>> length value will become negative. If that is the case, then just abort
> >>>> and ignore the option.
> >>>>
> >>>> In case an attacker uses a too short opt->len value, then garbage will
> >>>> be parsed, but that is protected by the unknown option handling and also
> >>>> the option parameter size checks.
> >>>>
> >>>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> >>>> ---
> >>>> net/bluetooth/l2cap_core.c | 6 ++++++
> >>>> 1 file changed, 6 insertions(+)
> >>>>
> >>>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >>>> index 77799e7d5a34..ccdc5c67d22a 100644
> >>>> --- a/net/bluetooth/l2cap_core.c
> >>>> +++ b/net/bluetooth/l2cap_core.c
> >>>> @@ -3337,6 +3337,8 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
> >>>>
> >>>> while (len >= L2CAP_CONF_OPT_SIZE) {
> >>>> len -= l2cap_get_conf_opt(&req, &type, &olen, &val);
> >>>> + if (len < 0)
> >>>> + break;
> >>>
> >>> <snip>
> >>>
> >>> Patch looks good to me, thanks for fixing this all up:
> >>
> >> it would be still good if we can get this verified by the reporter.
> >
> > The "reporter" seems to have disappeared once they reported this stuff,
> > so I would not count on them doing anything here, we asked numerous
> > times :(
> >
> > If the patches look correct, I recommend just merging it and I can
> > backport it to the stable releases and the distros can pick it up from
> > there.
>
> I think that I have Johan merge them into bluetooth-next first and let
> them sit a little so we can verify they do not break anything else.
> You can pick them up later into -stable if nobody complained and we
> didn’t break qualification.
Sounds good to me, thanks.
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer
2019-01-18 12:43 [PATCH] Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer Marcel Holtmann
2019-01-18 12:53 ` Greg KH
@ 2019-01-23 11:37 ` Johan Hedberg
1 sibling, 0 replies; 8+ messages in thread
From: Johan Hedberg @ 2019-01-23 11:37 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth, gregkh
Hi Marcel,
On Fri, Jan 18, 2019, Marcel Holtmann wrote:
> The function l2cap_get_conf_opt will return L2CAP_CONF_OPT_SIZE + opt->len
> as length value. The opt->len however is in control over the remote user
> and can be used by an attacker to gain access beyond the bounds of the
> actual packet.
>
> To prevent any potential leak of heap memory, it is enough to check that
> the resulting len calculation after calling l2cap_get_conf_opt is not
> below zero. A well formed packet will always return >= 0 here and will
> end with the length value being zero after the last option has been
> parsed. In case of malformed packets messing with the opt->len field the
> length value will become negative. If that is the case, then just abort
> and ignore the option.
>
> In case an attacker uses a too short opt->len value, then garbage will
> be parsed, but that is protected by the unknown option handling and also
> the option parameter size checks.
>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
> net/bluetooth/l2cap_core.c | 6 ++++++
> 1 file changed, 6 insertions(+)
Applied to bluetooth-next. Thanks.
Johan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer
@ 2023-12-20 13:16 Celia R L R Normand
0 siblings, 0 replies; 8+ messages in thread
From: Celia R L R Normand @ 2023-12-20 13:16 UTC (permalink / raw)
To: marcel; +Cc: gregkh, linux-bluetooth
Enviado do meu iPhone
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-12-20 13:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-18 12:43 [PATCH] Bluetooth: Verify that l2cap_get_conf_opt provides large enough buffer Marcel Holtmann
2019-01-18 12:53 ` Greg KH
2019-01-18 13:12 ` Marcel Holtmann
2019-01-18 15:00 ` Greg KH
2019-01-21 14:51 ` Marcel Holtmann
2019-01-21 15:09 ` Greg KH
2019-01-23 11:37 ` Johan Hedberg
-- strict thread matches above, loose matches on Subject: below --
2023-12-20 13:16 Celia R L R Normand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).