From owner-freebsd-current@FreeBSD.ORG Thu Aug 25 20:31:52 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3026A106566B; Thu, 25 Aug 2011 20:31:52 +0000 (UTC) (envelope-from jwd@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 04C678FC12; Thu, 25 Aug 2011 20:31:52 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p7PKVpgs080537; Thu, 25 Aug 2011 20:31:51 GMT (envelope-from jwd@freefall.freebsd.org) Received: (from jwd@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p7PKVp3T080536; Thu, 25 Aug 2011 20:31:51 GMT (envelope-from jwd) Date: Thu, 25 Aug 2011 20:31:51 +0000 From: John To: freebsd-current@freebsd.org, freebsd-fs@freebsd.org Message-ID: <20110825203151.GA61776@FreeBSD.org> References: <20110824212842.GA50140@FreeBSD.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110824212842.GA50140@FreeBSD.org> User-Agent: Mutt/1.4.2.3i Cc: Subject: Re: F_RDLCK lock to FreeBSD NFS server fails to R/O target file [PATCH] X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Aug 2011 20:31:52 -0000 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 >