2013-10-16 08:37:00

by Johan Hedberg

[permalink] [raw]
Subject: [PATCH 1/2] Bluetooth: Ignore A2MP data on non-BR/EDR links

From: Johan Hedberg <[email protected]>

The A2MP CID is only valid for BR/EDR transports. We should ignore A2MP
data on non-BR/EDR links and refuse to create an amp_mgr object.

Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/a2mp.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index fe32a33..efcd108 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -836,6 +836,9 @@ struct l2cap_chan *a2mp_channel_create(struct l2cap_conn *conn,
{
struct amp_mgr *mgr;

+ if (conn->hcon->type != ACL_LINK)
+ return NULL;
+
mgr = amp_mgr_create(conn, false);
if (!mgr) {
BT_ERR("Could not create AMP manager");
--
1.8.3.1



2013-10-16 09:10:54

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/2] Bluetooth: Ignore A2MP data on non-BR/EDR links

Hi Andrei,

>> The A2MP CID is only valid for BR/EDR transports. We should ignore A2MP
>> data on non-BR/EDR links and refuse to create an amp_mgr object.
>>
>> Signed-off-by: Johan Hedberg <[email protected]>
>> ---
>> net/bluetooth/a2mp.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
>> index fe32a33..efcd108 100644
>> --- a/net/bluetooth/a2mp.c
>> +++ b/net/bluetooth/a2mp.c
>> @@ -836,6 +836,9 @@ struct l2cap_chan *a2mp_channel_create(struct l2cap_conn *conn,
>> {
>> struct amp_mgr *mgr;
>>
>> + if (conn->hcon->type != ACL_LINK)
>> + return NULL;
>> +
>
> Have you experienced this ever happened?

this is how good software is written. You check that your input is valid first. Otherwise you open yourself up to vulnerabilities.

Regards

Marcel


2013-10-16 08:42:50

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] Bluetooth: Ignore SMP data on non-LE links

Hi Johan,

> The SMP CID is only defined for LE transports. Instead of returning an
> error from smp_sig_channel() in this case (which would cause a
> disconnection) just return 0 to ignore the data, which is consistent
> with the behavior for other unknown CIDs.
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> net/bluetooth/smp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel


2013-10-16 08:42:22

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/2] Bluetooth: Ignore A2MP data on non-BR/EDR links

Hi Johan,

> The A2MP CID is only valid for BR/EDR transports. We should ignore A2MP
> data on non-BR/EDR links and refuse to create an amp_mgr object.
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> net/bluetooth/a2mp.c | 3 +++
> 1 file changed, 3 insertions(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel


2013-10-16 08:54:47

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] Bluetooth: Ignore A2MP data on non-BR/EDR links

Hi Johan,

On Wed, Oct 16, 2013 at 11:37:00AM +0300, [email protected] wrote:
> From: Johan Hedberg <[email protected]>
>
> The A2MP CID is only valid for BR/EDR transports. We should ignore A2MP
> data on non-BR/EDR links and refuse to create an amp_mgr object.
>
> Signed-off-by: Johan Hedberg <[email protected]>
> ---
> net/bluetooth/a2mp.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
> index fe32a33..efcd108 100644
> --- a/net/bluetooth/a2mp.c
> +++ b/net/bluetooth/a2mp.c
> @@ -836,6 +836,9 @@ struct l2cap_chan *a2mp_channel_create(struct l2cap_conn *conn,
> {
> struct amp_mgr *mgr;
>
> + if (conn->hcon->type != ACL_LINK)
> + return NULL;
> +

Have you experienced this ever happened?

Best regards
Andrei Emeltchenko


2013-10-16 08:37:01

by Johan Hedberg

[permalink] [raw]
Subject: [PATCH 2/2] Bluetooth: Ignore SMP data on non-LE links

From: Johan Hedberg <[email protected]>

The SMP CID is only defined for LE transports. Instead of returning an
error from smp_sig_channel() in this case (which would cause a
disconnection) just return 0 to ignore the data, which is consistent
with the behavior for other unknown CIDs.

Signed-off-by: Johan Hedberg <[email protected]>
---
net/bluetooth/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 463e50c..fc200e0 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -856,7 +856,7 @@ int smp_sig_channel(struct l2cap_conn *conn, struct sk_buff *skb)

if (hcon->type != LE_LINK) {
kfree_skb(skb);
- return -ENOTSUPP;
+ return 0;
}

if (skb->len < 1) {
--
1.8.3.1