From owner-freebsd-fs@FreeBSD.ORG Fri Aug 26 19:21:09 2011 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EDAF8106566C; Fri, 26 Aug 2011 19:21:08 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-annu.mail.uoguelph.ca (esa-annu.mail.uoguelph.ca [131.104.91.36]) by mx1.freebsd.org (Postfix) with ESMTP id 78AEB8FC13; Fri, 26 Aug 2011 19:21:08 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AqAEAIvxV06DaFvO/2dsb2JhbAA7CBaENqRRgUABAQEBAwEBARoGBCcgBgUbGAICDRkCKQEJJgYIBwQBHASHVahukWiBLIF/ghCBEQSRCoIQiD2IXw X-IronPort-AV: E=Sophos;i="4.68,286,1312171200"; d="scan'208";a="132379155" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-annu-pri.mail.uoguelph.ca with ESMTP; 26 Aug 2011 15:21:07 -0400 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id 8D316B3F1F; Fri, 26 Aug 2011 15:21:07 -0400 (EDT) Date: Fri, 26 Aug 2011 15:21:07 -0400 (EDT) From: Rick Macklem To: John Message-ID: <470466580.417415.1314386467564.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20110825203151.GA61776@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [172.17.91.203] X-Mailer: Zimbra 6.0.10_GA_2692 (ZimbraWebClient - FF3.0 (Win)/6.0.10_GA_2692) Cc: freebsd-fs@freebsd.org, freebsd-current@freebsd.org Subject: Re: F_RDLCK lock to FreeBSD NFS server fails to R/O target file [PATCH] X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Aug 2011 19:21:09 -0000 John wrote: > 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 took a look at it and it seemed mostly ok. However, please note that I am not familiar with the NLM code and try to avoid it like the plague.:-) One place I would suggest might want to be changed is the nlm_do_unlock() case. I don't think any file permission checking is needed for an unlock and it seems like it might fail when the called has VWRITE but not VREAD permissions on the file? Leaving a file locked is not a good situation. I would just not even do the VOP_ACCESS() call for that case. Maybe pass accmode == 0 into nlm_get_vfs_state() to indicate "skip the VOP_ACCESS() call". I think that this patch might be a little risky to put into head at this stage of the release cycle so, personally, I'd wait until after the 9.0 release before I'd look at committing it. Others might feel it's ok to go in now? rick > 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 > > > _______________________________________________ > freebsd-fs@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-fs > To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org"