* [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
@ 2010-08-19 11:06 Luiz Augusto von Dentz
2010-08-20 10:55 ` Ville Tervo
2010-10-28 13:07 ` Gustavo F. Padovan
0 siblings, 2 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2010-08-19 11:06 UTC (permalink / raw)
To: linux-bluetooth; +Cc: ville.tervo
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
This cause 'No Bonding' to be used if userspace has not yet been paired
with remote device since the l2cap socket used to create the rfcomm
session does not have any security level set.
Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
---
net/bluetooth/rfcomm/core.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 7dca91b..08f5757 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -79,7 +79,10 @@ static void rfcomm_make_uih(struct sk_buff *skb, u8 addr);
static void rfcomm_process_connect(struct rfcomm_session *s);
-static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err);
+static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
+ bdaddr_t *dst,
+ u8 sec_level,
+ int *err);
static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdaddr_t *dst);
static void rfcomm_session_del(struct rfcomm_session *s);
@@ -402,7 +405,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
s = rfcomm_session_get(src, dst);
if (!s) {
- s = rfcomm_session_create(src, dst, &err);
+ s = rfcomm_session_create(src, dst, d->sec_level, &err);
if (!s)
return err;
}
@@ -680,7 +683,10 @@ static void rfcomm_session_close(struct rfcomm_session *s, int err)
rfcomm_session_put(s);
}
-static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err)
+static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
+ bdaddr_t *dst,
+ u8 sec_level,
+ int *err)
{
struct rfcomm_session *s = NULL;
struct sockaddr_l2 addr;
@@ -705,6 +711,7 @@ static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst
sk = sock->sk;
lock_sock(sk);
l2cap_pi(sk)->imtu = l2cap_mtu;
+ l2cap_pi(sk)->sec_level = sec_level;
if (l2cap_ertm)
l2cap_pi(sk)->mode = L2CAP_MODE_ERTM;
release_sock(sk);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
@ 2010-08-03 7:26 Luiz Augusto von Dentz
2010-08-03 7:59 ` Marcel Holtmann
0 siblings, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2010-08-03 7:26 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
This cause 'No Bonding' to be used if userspace has not yet been paired
with remote device since the l2cap socket used to create the rfcomm
session does not have any security level set.
Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
---
net/bluetooth/rfcomm/core.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 7dca91b..08f5757 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -79,7 +79,10 @@ static void rfcomm_make_uih(struct sk_buff *skb, u8 addr);
static void rfcomm_process_connect(struct rfcomm_session *s);
-static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err);
+static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
+ bdaddr_t *dst,
+ u8 sec_level,
+ int *err);
static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdaddr_t *dst);
static void rfcomm_session_del(struct rfcomm_session *s);
@@ -402,7 +405,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
s = rfcomm_session_get(src, dst);
if (!s) {
- s = rfcomm_session_create(src, dst, &err);
+ s = rfcomm_session_create(src, dst, d->sec_level, &err);
if (!s)
return err;
}
@@ -680,7 +683,10 @@ static void rfcomm_session_close(struct rfcomm_session *s, int err)
rfcomm_session_put(s);
}
-static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err)
+static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
+ bdaddr_t *dst,
+ u8 sec_level,
+ int *err)
{
struct rfcomm_session *s = NULL;
struct sockaddr_l2 addr;
@@ -705,6 +711,7 @@ static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst
sk = sock->sk;
lock_sock(sk);
l2cap_pi(sk)->imtu = l2cap_mtu;
+ l2cap_pi(sk)->sec_level = sec_level;
if (l2cap_ertm)
l2cap_pi(sk)->mode = L2CAP_MODE_ERTM;
release_sock(sk);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
2010-08-03 7:26 Luiz Augusto von Dentz
@ 2010-08-03 7:59 ` Marcel Holtmann
2010-08-03 8:27 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2010-08-03 7:59 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
> This cause 'No Bonding' to be used if userspace has not yet been paired
> with remote device since the l2cap socket used to create the rfcomm
> session does not have any security level set.
>
> Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> ---
> net/bluetooth/rfcomm/core.c | 13 ++++++++++---
> 1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index 7dca91b..08f5757 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -79,7 +79,10 @@ static void rfcomm_make_uih(struct sk_buff *skb, u8 addr);
>
> static void rfcomm_process_connect(struct rfcomm_session *s);
>
> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err);
> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
> + bdaddr_t *dst,
> + u8 sec_level,
> + int *err);
> static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdaddr_t *dst);
> static void rfcomm_session_del(struct rfcomm_session *s);
>
> @@ -402,7 +405,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
>
> s = rfcomm_session_get(src, dst);
> if (!s) {
> - s = rfcomm_session_create(src, dst, &err);
> + s = rfcomm_session_create(src, dst, d->sec_level, &err);
> if (!s)
> return err;
> }
> @@ -680,7 +683,10 @@ static void rfcomm_session_close(struct rfcomm_session *s, int err)
> rfcomm_session_put(s);
> }
>
> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err)
> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
> + bdaddr_t *dst,
> + u8 sec_level,
> + int *err)
> {
> struct rfcomm_session *s = NULL;
> struct sockaddr_l2 addr;
> @@ -705,6 +711,7 @@ static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst
> sk = sock->sk;
> lock_sock(sk);
> l2cap_pi(sk)->imtu = l2cap_mtu;
> + l2cap_pi(sk)->sec_level = sec_level;
> if (l2cap_ertm)
> l2cap_pi(sk)->mode = L2CAP_MODE_ERTM;
> release_sock(sk);
I see what you are trying to do here, but why is this needed? Shouldn't
it be already correctly set for PSM 3?
Regards
Marcel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
2010-08-03 7:59 ` Marcel Holtmann
@ 2010-08-03 8:27 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2010-08-03 8:27 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
On Tue, Aug 3, 2010 at 10:59 AM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Luiz,
>
>> This cause 'No Bonding' to be used if userspace has not yet been paired
>> with remote device since the l2cap socket used to create the rfcomm
>> session does not have any security level set.
>>
>> Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
>> ---
>> =A0net/bluetooth/rfcomm/core.c | =A0 13 ++++++++++---
>> =A01 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
>> index 7dca91b..08f5757 100644
>> --- a/net/bluetooth/rfcomm/core.c
>> +++ b/net/bluetooth/rfcomm/core.c
>> @@ -79,7 +79,10 @@ static void rfcomm_make_uih(struct sk_buff *skb, u8 a=
ddr);
>>
>> =A0static void rfcomm_process_connect(struct rfcomm_session *s);
>>
>> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdad=
dr_t *dst, int *err);
>> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bdaddr_t *dst,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 u8 sec_level,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int *err);
>> =A0static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdadd=
r_t *dst);
>> =A0static void rfcomm_session_del(struct rfcomm_session *s);
>>
>> @@ -402,7 +405,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, b=
daddr_t *src, bdaddr_t *dst,
>>
>> =A0 =A0 =A0 s =3D rfcomm_session_get(src, dst);
>> =A0 =A0 =A0 if (!s) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 s =3D rfcomm_session_create(src, dst, &err);
>> + =A0 =A0 =A0 =A0 =A0 =A0 s =3D rfcomm_session_create(src, dst, d->sec_l=
evel, &err);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!s)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return err;
>> =A0 =A0 =A0 }
>> @@ -680,7 +683,10 @@ static void rfcomm_session_close(struct rfcomm_sess=
ion *s, int err)
>> =A0 =A0 =A0 rfcomm_session_put(s);
>> =A0}
>>
>> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdad=
dr_t *dst, int *err)
>> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bdaddr_t *dst,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 u8 sec_level,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int *err)
>> =A0{
>> =A0 =A0 =A0 struct rfcomm_session *s =3D NULL;
>> =A0 =A0 =A0 struct sockaddr_l2 addr;
>> @@ -705,6 +711,7 @@ static struct rfcomm_session *rfcomm_session_create(=
bdaddr_t *src, bdaddr_t *dst
>> =A0 =A0 =A0 sk =3D sock->sk;
>> =A0 =A0 =A0 lock_sock(sk);
>> =A0 =A0 =A0 l2cap_pi(sk)->imtu =3D l2cap_mtu;
>> + =A0 =A0 l2cap_pi(sk)->sec_level =3D sec_level;
>> =A0 =A0 =A0 if (l2cap_ertm)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 l2cap_pi(sk)->mode =3D L2CAP_MODE_ERTM;
>> =A0 =A0 =A0 release_sock(sk);
>
> I see what you are trying to do here, but why is this needed? Shouldn't
> it be already correctly set for PSM 3?
I couldn't find any special check for PSM 3, but I guess it would make
sense to make sure it always set the security level from dlc which
triggered the session, doesn't it? As it is now it first triggers No
Bonding and then when we actually connect the dlc it might have a
different sec_level which might triggers another round of io
capability negotiation, not only this doesn't work always, because
there is a bug in bluetoothd that won't invalidate/remove stored link
keys once we got a new one that is not supposed to be stored (No
Bonding), but IMO much simpler too. Maybe we can do as SDP which seems
to do the check in l2cap.c:l2cap_do_connect, but for rfcomm we
actually have to look in the dlcs to check witch security level should
be use.
--=20
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-28 13:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-19 11:06 [PATCH] bluetooth: fix not setting security level when creating a rfcomm session Luiz Augusto von Dentz
2010-08-20 10:55 ` Ville Tervo
2010-10-28 13:07 ` Gustavo F. Padovan
-- strict thread matches above, loose matches on Subject: below --
2010-08-03 7:26 Luiz Augusto von Dentz
2010-08-03 7:59 ` Marcel Holtmann
2010-08-03 8:27 ` Luiz Augusto von Dentz
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).