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

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

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

Hi All,

Ershov Yuri (EXT-Teleca/RussianFed) wrote:
> Hi Ville,
>
> Ville Tervo wrote:
>> Hi Yuri,
>>
>> On Wed, Dec 08, 2010 at 01:52:09PM +0300, Yuri Ershov wrote:
>>
>>
>>   
>>>>>> 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

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