Re: Huge race in lockd for async lock requests?

J. Bruce Fields
2009-05-28T20:05:23Z

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.