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

Hi All,

Ershov Yuri (EXT-Teleca/RussianFed) wrote:

Hi Ville,

Ville Tervo wrote:

quoted

Hi Yuri,

On Wed, Dec 08, 2010 at 01:52:09PM +0300, Yuri Ershov wrote:


  

quoted

quoted

quoted

quoted

So in which situations (n == p), or (p == p->next)? That should happen only
when p is the only element in the list, then p == head, right?
          

The (n == p) is in situation, when sk is unlinked by task
responsible for handling connect/disconnect requests while the
"bt_accept_dequeue". This condition is indirect checking of sk
validity.
        

Why not using a list lock here instead? Fits a way better.

      

Yes, it's better. I tried to use the locks in this function, but it
slows down the task handling connect/disconnect/etc. events and the
task skips some events from fast clients.

    

What kind of problems you exactly got with locks? Maybe they should be fixed
also.

  

The sequence with locks is the following (let's consider 
connect/disconnect events only):

Clients                                   bt main 
task                                bt accept task
    |                                                  
|                                                      |
    |                                                  
|                                           schedule_timeout()
connect                                 
sig_channel                                            |
    |                                                  
|                                                       |
    |                                            wake_up               
                                | 
                                                  
    |                                                  
|                                                       |
disconnect                            
sig_channel                                  bt_accept_deque
    |                                                  |               
                                        |
    |                                                  |               
                                      lock
    |                                       wait for 
lock                                              |
    |                                                  
|                                                       |
connect                                  skip event                   
                      unlock

etc.
                                                                     
        
So when I use several clients, skipping events becomes appreciable. I 
found only one way to leave "bt main task" (very fast) and "bt accept 
task" (rather slow) synchronized - skip handling of invalid sockets.

It is just to remind, if the patch is not rejected. May I ask you to 
comment this?

Thanks,
Yuri