Re: Huge race in lockd for async lock requests?

["J. Bruce Fields" <bfields@fieldses.org>]

On Wed, May 20, 2009 at 10:37:05AM -0600, Rob Gardner wrote:
> Tom Talpey wrote:
>> At 02:55 AM 5/20/2009, Rob Gardner wrote:
>>   
>>> Tom Talpey wrote:
>>>     
>>>> At 04:43 PM 5/19/2009, Rob Gardner wrote:
>>>>       
>>>>> I've got a question about lockd in conjunction with a filesystem 
>>>>> that provides its own (async) locking.
>>>>>
>>>>> After nlmsvc_lock() calls vfs_lock_file(), it seems to be that we 
>>>>> might get the async callback (nlmsvc_grant_deferred) at any time. 
>>>>> What's to stop it from arriving before we even put the block on 
>>>>> the nlm_block list? If this happens, then nlmsvc_grant_deferred() 
>>>>> will print "grant for unknown block" and then we'll wait forever 
>>>>> for a grant that will never come.
>>>>>         
>>>> Yes, there's a race but the client will retry every 30 seconds, so it won't
>>>> wait forever.
>>>>       
>>> OK, a blocking lock request will get retried in 30 seconds and work 
>>> out "ok". But a non-blocking request will get in big trouble. Let's 
>>> say the     
>>
>> A non-blocking lock doesn't request, and won't get, a callback. So I
>> don't understand...
>>
>>   
>
> What do you mean a non-blocking lock doesn't request? Remember that I'm  
> dealing with a filesystem that provides its own locking functions via  
> file->f_op->lock(). Such a filesystem might easily defer a non-blocking  
> lock request and invoke the callback later. At least I don't know of any  
> rule that says that it can't do this, and clearly the code expects this  
> possibility:
>
>              case FILE_LOCK_DEFERRED:
>                        if (wait)
>                                break;
>                        /* Filesystem lock operation is in progress
>                           Add it to the queue waiting for callback */
>                        ret = nlmsvc_defer_lock_rqst(rqstp, block);
>
>
>>> callback is invoked immediately after the vfs_lock_file call returns  
>>> FILE_LOCK_DEFERRED. At this point, the block is not on the nlm_block  
>>> list, so the callback routine will not be able to find it and mark it 
>>> as granted. Then nlmsvc_lock() will call nlmsvc_defer_lock_rqst(), 
>>> put the block on the nlm_block list, and eventually the request will 
>>> timeout and the client will get lck_denied. Meanwhile, the lock has 
>>> actually been granted, but nobody knows about it.
>>>     
>>
>> Yes, this can happen, I've seen it too. Again, it's a bug in the protocol
>> more than a bug in the clients. 
> It looks to me like a bug in the server. The server must be able to deal  
> with async filesystem callbacks happening at any time, however 
> inconvenient.

Absolutely, if that's possible then it's a server bug.

--b.