headers
Frédéric Danis | 20 Jun 2012 11:02
Picon

[PATCH] sdp: Prevent duplicate records registration

Check if a record with same UUID and protocol descriptor already exists
before adding new record to server
---

When BlueZ is built with --enable-pnat option, it provides DUN support on RFComm
port 1.
When current version of oFono is started, it also provides DUN support on same
port.
So, we get to 2 SDP records for same UUID and RFComm port.
This patch prevents this.

 src/sdpd-service.c |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 39e05ab..9e8b90c 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
 <at>  <at>  -235,6 +235,9  <at>  <at>  int add_record_to_server(const bdaddr_t *src, sdp_record_t *rec)
 {
 	sdp_data_t *data;
 	sdp_list_t *pattern;
+	sdp_list_t *record;
+	sdp_list_t *protos;
+	int port = -1, psm = -1;

 	if (rec->handle == 0xffffffff) {
 		rec->handle = sdp_next_handle();
 <at>  <at>  -245,6 +248,38  <at>  <at>  int add_record_to_server(const bdaddr_t *src, sdp_record_t *rec)
 			return -EEXIST;
(Continue reading)

Permalink | Reply |
headers
Johan Hedberg | 20 Jun 2012 11:35
Picon
Gravatar

Re: [PATCH] sdp: Prevent duplicate records registration

Hi Frédéric,

On Wed, Jun 20, 2012, Frédéric Danis wrote:
> Check if a record with same UUID and protocol descriptor already exists
> before adding new record to server
> ---
> 
> When BlueZ is built with --enable-pnat option, it provides DUN support on RFComm
> port 1.
> When current version of oFono is started, it also provides DUN support on same
> port.
> So, we get to 2 SDP records for same UUID and RFComm port.
> This patch prevents this.
> 
> 
>  src/sdpd-service.c |   35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/src/sdpd-service.c b/src/sdpd-service.c
> index 39e05ab..9e8b90c 100644
> --- a/src/sdpd-service.c
> +++ b/src/sdpd-service.c
>  <at>  <at>  -235,6 +235,9  <at>  <at>  int add_record_to_server(const bdaddr_t *src, sdp_record_t *rec)
>  {
>  	sdp_data_t *data;
>  	sdp_list_t *pattern;
> +	sdp_list_t *record;
> +	sdp_list_t *protos;
> +	int port = -1, psm = -1;
>
(Continue reading)

Permalink | Reply |
headers
Frederic Danis | 20 Jun 2012 15:09
Picon

Re: [PATCH] sdp: Prevent duplicate records registration

Hello Johan,

On 20/06/2012 11:35, Johan Hedberg wrote:
> Hi Frédéric,
>
> On Wed, Jun 20, 2012, Frédéric Danis wrote:
>> Check if a record with same UUID and protocol descriptor already exists
>> before adding new record to server
>> ---
>>
>> When BlueZ is built with --enable-pnat option, it provides DUN support on RFComm
>> port 1.
>> When current version of oFono is started, it also provides DUN support on same
>> port.
>> So, we get to 2 SDP records for same UUID and RFComm port.
>> This patch prevents this.
>>
>
> My initial reaction is that I don't think this is something that needs
> to be part of the SDP server. The admin of the system should be smart
> enough to not try to configure to identical&  conflicting services.
>
> Also, the RFCOMM server socket code in the kernel should already give an
> error if binding to the same channel is attempted twice, so this would
> look like a bug in one of the DUN implementations that they do not
> unregister their service record when binding the server socket fails. So
> simply fixing this bug would also make sure that two service records
> aren't present (though it would remove the existence of the clueless
> admin ;)
>
(Continue reading)

Permalink | Reply |
headers
Luiz Augusto von Dentz | 21 Jun 2012 10:36
Picon

Re: [PATCH] sdp: Prevent duplicate records registration

Hi Johan,

On Wed, Jun 20, 2012 at 12:35 PM, Johan Hedberg
<johan.hedberg@...> wrote:

> Also, the RFCOMM server socket code in the kernel should already give an
> error if binding to the same channel is attempted twice, so this would
> look like a bug in one of the DUN implementations that they do not
> unregister their service record when binding the server socket fails. So
> simply fixing this bug would also make sure that two service records
> aren't present (though it would remove the existence of the clueless
> admin ;)

Good point, and should actually work for L2CAP as well, apparently
__rfcomm_get_sock_by_addr don't check BDADDR_ANY:

		if (rfcomm_pi(sk)->channel == channel &&
				!bacmp(&bt_sk(sk)->src, src))
			break;

So the question is, should we check in every adapter if there is
listen socket for the given channel? This complicates a little bit
more the code, the other option is to oFono don't do the socket
management itself, this might be possible with org.bluez.Telephony, so
just register an agent for DUN as it will be for HFP, bluetoothd takes
care of handling the connection and authorization and when its done
contact oFono.

--

-- 
Luiz Augusto von Dentz
(Continue reading)

Permalink | Reply |
headers
Johan Hedberg | 29 Jun 2012 15:35
Picon
Gravatar

Re: [PATCH] sdp: Prevent duplicate records registration

Hi Luiz,

On Thu, Jun 21, 2012, Luiz Augusto von Dentz wrote:
> On Wed, Jun 20, 2012 at 12:35 PM, Johan Hedberg
<johan.hedberg@...> wrote:
> 
> > Also, the RFCOMM server socket code in the kernel should already give an
> > error if binding to the same channel is attempted twice, so this would
> > look like a bug in one of the DUN implementations that they do not
> > unregister their service record when binding the server socket fails. So
> > simply fixing this bug would also make sure that two service records
> > aren't present (though it would remove the existence of the clueless
> > admin ;)
> 
> Good point, and should actually work for L2CAP as well, apparently
> __rfcomm_get_sock_by_addr don't check BDADDR_ANY:
> 
> 		if (rfcomm_pi(sk)->channel == channel &&
> 				!bacmp(&bt_sk(sk)->src, src))
> 			break;
> 
> So the question is, should we check in every adapter if there is
> listen socket for the given channel?

I think the kernel should return an error is bind is tried with
BDADDR_ANY more than once for the same PSM or RFCOMM channel.

> This complicates a little bit more the code, the other option is to
> oFono don't do the socket management itself, this might be possible
> with org.bluez.Telephony, so just register an agent for DUN as it will
(Continue reading)

Permalink | Reply |
headers
Luiz Augusto von Dentz | 29 Jun 2012 15:43
Picon

Re: [PATCH] sdp: Prevent duplicate records registration

Hi Johan,

On Fri, Jun 29, 2012 at 4:35 PM, Johan Hedberg
<johan.hedberg@...> wrote:
>> This complicates a little bit more the code, the other option is to
>> oFono don't do the socket management itself, this might be possible
>> with org.bluez.Telephony, so just register an agent for DUN as it will
>> be for HFP, bluetoothd takes care of handling the connection and
>> authorization and when its done
>
> Isn't that exactly what the last two patches (for DUN and SAP) from the
> telephony patch set do?

I guess so, but I didn't remember those in the original series so it
might be introduced latter, Frederic?

--

-- 
Luiz Augusto von Dentz
Permalink | Reply |
headers
Frederic Danis | 29 Jun 2012 15:52
Picon

Re: [PATCH] sdp: Prevent duplicate records registration

Hello Luiz,

On 29/06/2012 15:43, Luiz Augusto von Dentz wrote:
> Hi Johan,
>
> On Fri, Jun 29, 2012 at 4:35 PM, Johan Hedberg
<johan.hedberg@...> wrote:
>>> This complicates a little bit more the code, the other option is to
>>> oFono don't do the socket management itself, this might be possible
>>> with org.bluez.Telephony, so just register an agent for DUN as it will
>>> be for HFP, bluetoothd takes care of handling the connection and
>>> authorization and when its done
>>
>> Isn't that exactly what the last two patches (for DUN and SAP) from the
>> telephony patch set do?
>
> I guess so, but I didn't remember those in the original series so it
> might be introduced latter, Frederic?
>
You are right, this was introduced in v10 of patches.

--

-- 
Frederic Danis                            Open Source Technology Center
frederic.danis@...                              Intel Corporation
Permalink | Reply |

Gmane