Hi Andrzej,
>>> When binding RFCOMM socket we should only check if there is another
>>> socket bound or listening on the same channel number. In other case,
>>> it won't be possible to bind/listen on a channel in case we have
>>> connection made to remote device on the same channel number.
>>
>> since this has been used for years now, you need to be more specific on when this fails.
>
> It's quite simple: create one socket and connect on channel X, then
> create another socket and try to bind on channel X. Event though we
> don't have listening socket on channel X yet, it will fail with
> EADDRINUSE since rfcomm_sock_bind looks for *any* socket on specified
> channel and doesn't care if it's bound/listening on local channel or
> just connected to remote channel (in which case it should not fail).
>
> Is it specific enough?
can we add a test case to rfcomm-tester for this first.
>>> Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@xxxxxxxxx>
>>> ---
>>> net/bluetooth/rfcomm/sock.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
>>> index 00573fb..9912e23 100644
>>> --- a/net/bluetooth/rfcomm/sock.c
>>> +++ b/net/bluetooth/rfcomm/sock.c
>>> @@ -331,6 +331,7 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
>>> {
>>> struct sockaddr_rc *sa = (struct sockaddr_rc *) addr;
>>> struct sock *sk = sock->sk;
>>> + struct sock *sk1;
>>> int err = 0;
>>>
>>> BT_DBG("sk %p %pMR", sk, &sa->rc_bdaddr);
>>> @@ -352,7 +353,9 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
>>>
>>> write_lock(&rfcomm_sk_list.lock);
>>>
>>> - if (sa->rc_channel && __rfcomm_get_sock_by_addr(sa->rc_channel, &sa->rc_bdaddr)) {
>>> + sk1 = __rfcomm_get_sock_by_addr(sa->rc_channel, &sa->rc_bdaddr);
>>> + if (sa->rc_channel && sk1 && (sk1->sk_state == BT_BOUND ||
>>> + sk1->sk_state == BT_LISTEN)) {
>>> err = -EADDRINUSE;
>>
>> can we find a better name than sk1 here.
>
> Something like 'existing_sk'? Or just 'oldsk'? I have no clue how to
> make a meaningful name here.
I wonder if you should not just fix __rfcomm_get_sock_by_addr to check only for BT_BOUND and BT_LISTEN.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
