Re: [PATCH] bluetooth: Fix NULL pointer dereference issue

[Yuri Ershov <ext-yuri.ershov@nokia.com>]

[-- Attachment #1: Type: text/plain, Size: 1732 bytes --]

Hello Gustavo,

The problem appears in case of multiple connect-transfer-disconnect 
sequence (e.g. by using l2test). The conditions are the following:
There are 2 BT devices. The first one listens and receives (l2test -r), 
the second one makes "connect-disconnect-connect..." sequence (l2test -c 
-b 1000 -i hci0 -P 10 <addr>). After some time this will cause the race 
between functions bt_accept_dequeue and l2cap_chan_del. The fail sequence:

struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
{
...
    list_for_each_safe(p, n, &bt_sk(parent)->accept_q) {
        sk = (struct sock *) list_entry(p, struct bt_sock, accept_q);

        lock_sock(sk);



In this time the function l2cap_chan_del sets the socket state to 
BT_CLOSED, unlinks and kills the socket.



        /* FIXME: Is this check still needed */
        if (sk->sk_state == BT_CLOSED) {
            release_sock(sk);
            bt_accept_unlink(sk);
            continue;
        }

...

        release_sock(sk);
    }
    return NULL;
}
 

Regards,
Yuri



ext Gustavo F. Padovan wrote:
> Hi Yuri,
>
> * Yuri Ershov <ext-yuri.ershov@nokia.com> [2010-10-21 20:08:58 +0400]:
>
>   
>> This patch fixes NULL pointer dereference at running test with
>> connect-transfer-disconnect in loop. Sometimes sk_state is 
>> BT_CLOSED and sk_refcnt equal to 0, so there is oops in 
>> bt_accept_unlink. In normal case removed block is not used.
>>     
>
> Question here is: Why sk_refcnt is 0 at that point of the code?  The
> socket should be destroyed if it ref is 0, but it wasn't, so something
> in another point of the code went is wrong. "Sometimes" is not a good
> description of the problem, you have to show why that happened.
>
>   


[-- Attachment #2: Type: text/html, Size: 2746 bytes --]