Re: F_RDLCK lock to FreeBSD NFS server fails to R/O target file [PATCH]

After pondering the best way to allow the VOP_ACCESS() call to
only query for the permissions really needed, I've come up with
a patch that minimally adds one parameter to the nlm_get_vfs_state()
function call with the lock type from the original argp.

http://people.freebsd.org/~jwd/nlm_prot_impl.c.accmode.patch

I'd appreciate a review and seeing what might be required to commit
this prior to 9 release.

Thanks,
John

----- John's Original Message -----
> Hi Fellow NFS'ers,
> 
>    I believe I have found the problem we've been having with read locks
> while attaching to a FreeBSD NFS server.
> 
>    In sys/nlm/nlm_prot_impl.c, function nlm_get_vfs_state(), there is a call
> to VOP_ACCESS() as follows:
> 
>         /*
>          * Check cred.
>          */
>         NLM_DEBUG(3, "nlm_get_vfs_state(): Calling VOP_ACCESS(VWRITE) with cred->cr_uid=%d\n",cred->cr_uid);
>         error = VOP_ACCESS(vs->vs_vp, VWRITE, cred, curthread);
>         if (error) {
>                 NLM_DEBUG(3, "nlm_get_vfs_state(): caller_name = %s VOP_ACCESS() returns %d\n",
>                 host->nh_caller_name, error);
>                 goto out;
>         }
> 
>   The file being accessed is read only to the user, and open()ed with
> O_RDONLY. The lock being requested is for a read.
> 
> fd = open(filename, O_RDONLY, 0);
> ...
> 
> lblk.l_type  = F_RDLCK;
> lblk.l_start = 0;
> lblk.l_whence= SEEK_SET;
> lblk.l_len   = 0;
> lblk.l_pid   = 0;
> rc = fcntl(fd, F_SETLK, &lblk);
> 
>    Running the above from a remote system, the lock call fails with
> errno set to ENOLCK. Given  cred->cr_uid comes in as 227 which is
> my uid on the remote system. Since the file is R/O to me, and the
> VOP_ACCESS() is asking for VWRITE, it fails with errno 13, EACCES,
> Permission denied.
> 
>    The above operations work correctly to some of our other
> favorite big-name nfs vendors :-)
> 
>    Opinions on the "correct" way to fix this?
> 
> 1. Since we're only asking for a read lock, why do we need to ask
>    for VWRITE? I may not understand an underlying requirement for
>    the VWRITE so please feel free to educate me if needed.
> 
>    Something like:  request == F_RDLCK ? VREAD : VWRITE
>    (need to figure out where to get the request from in this context).
> 
> 2. Attempt VWRITE, fallback to VREAD... seems off to me though.
> 
> 3. Other?
> 
>    I appreciate any thoughts on this.
> 
> Thanks,
> John
> 
>    While they might not follow style(9) completely, I've uploaded
> my patch to nlm_prot.impl.c with the NLM_DEBUG() calls i've added.
> I'd appreciate it if someone would consider committing them so
> who ever debugs this file next will have them available.
> 
> http://people.freebsd.org/~jwd/nlm_prot_impl.c.patch
>