RFC: NLM patch that is more permissive w.r.t. F_RDLCK and F_UNLCK

[John De]

Hi Rick,

   We have the patch installed and will start running some tests
against it today.

   I appreciate the time you & others have invested in this patch.

Thanks,
John


----- Rick Macklem's Original Message -----
> I have been working on getting a patch for the NLM, originally
> submitted by John De (jwd@) ready to commit to -current/head.
> The original patch fixes the case where a client attempts a read
> lock, but the uid on the client only has read access for the file.
> (This is allowed by POSIX and local file locking, from what I can see.)
> 
> I did make one change to his patch, which is to fall back to a test
> for write access when read access doesn't exist. This is a little
> weird, but since the unpatched code always checked for write access,
> this ensures that it will still work if it did without the patch.
> 
> While testing I also spotted a problem where, if file permissions are
> changed between a lock and an unlock, the unlock could fail, resulting
> in the file being left locked "forever".
> 
> When discussing this off-list, Zack responded with the following, which
> I have added to the patch. (ie. Being permissive w.r.t. unlock and blocking
> lock cancellation, so that a lock doesn't get left "forever".)
> Zack Kirsch wrote (re-posted with his permission):
> > Hey Rick, Doug,
> >
> > I completely agree. I've already implemented this change at Isilon,
> > where unlock passes a flag into nlm_get_vfs_state(). If the flag
> > is 0, then it causes the following to be skipped:
> > * VFS_CHECKEXP (because you still want to allow unlocking even
> >   though exports have changed)
> > * read-only checks
> > * svc_getcred (because the cred isn't needed)
> > * Any cred checks
> > * VOP_ACCESS call
> 
> Does anyone see a problem with this?
> (There could be a security argument against it, but I will simply note that
>  there is really no security against faulty clients in NFSv3 using AUTH_SYS
>  and, since clients normally check that an open of the appropriate type exists
>  in the client before attempting the locking operation against the server, 
>  if the client is "trusted" then this shouldn't be an issue.)
> 
> The patch is at:
>   http://people.freebsd.org/~rmacklem/nlmrlock.patch
> 
> Please comment on the above or the patch.
> 
> Thanks, rick
>