Date: Mon, 5 Dec 2011 20:36:28 -0500 (EST) From: Rick Macklem <rmacklem@uoguelph.ca> To: John <jwd@freebsd.org> 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] Message-ID: <1318843827.953380.1323135388258.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20110825203151.GA61776@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
------=_Part_953379_5444943.1323135388257 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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'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. > > I think I prefer this approach because it will never fail unless it would fail without the patch. For the case where the client uid has write but not read permission and attempts an F_RDLCK, I believe your patch would fail the request whereas it will succeed without your patch. (Although I'll admit I didn't actually test this.;-) Doing VOP_ACCESS(..VWRITE..) first and only doing a VOP_ACCESS(..VREAD..) if the VOP_ACCESS(..VWRITE..) fails will preserve current behaviour fot the non-failing case, but allow an F_RDLCK for uids with read-only access. I've attached a patch that does this variant. John, could you test this patch? And does anyone have an opinion w.r.t. which variant of the patch is more appropriate? Thanks in advance for your help, rick > > 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" ------=_Part_953379_5444943.1323135388257 Content-Type: text/x-patch; name=nlmrlock.patch Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename=nlmrlock.patch LS0tIG5sbS9ubG1fcHJvdF9pbXBsLmMuc2F2CTIwMTEtMTItMDUgMTA6NTk6MDAuMDAwMDAwMDAw IC0wNTAwCisrKyBubG0vbmxtX3Byb3RfaW1wbC5jCTIwMTEtMTItMDUgMjA6MDk6MDMuMDAwMDAw MDAwIC0wNTAwCkBAIC0xNzc0LDcgKzE3NzQsNyBAQCBzdHJ1Y3QgdmZzX3N0YXRlIHsKIAogc3Rh dGljIGludAogbmxtX2dldF92ZnNfc3RhdGUoc3RydWN0IG5sbV9ob3N0ICpob3N0LCBzdHJ1Y3Qg c3ZjX3JlcSAqcnFzdHAsCi0gICAgZmhhbmRsZV90ICpmaHAsIHN0cnVjdCB2ZnNfc3RhdGUgKnZz KQorICAgIGZoYW5kbGVfdCAqZmhwLCBzdHJ1Y3QgdmZzX3N0YXRlICp2cywgYm9vbF90IGV4Y2x1 c2l2ZSkKIHsKIAlpbnQgZXJyb3IsIGV4ZmxhZ3M7CiAJc3RydWN0IHVjcmVkICpjcmVkID0gTlVM TCwgKmNyZWRhbm9uOwpAQCAtMTgxNiw2ICsxODE2LDggQEAgbmxtX2dldF92ZnNfc3RhdGUoc3Ry dWN0IG5sbV9ob3N0ICpob3N0LAogCSAqIENoZWNrIGNyZWQuCiAJICovCiAJZXJyb3IgPSBWT1Bf QUNDRVNTKHZzLT52c192cCwgVldSSVRFLCBjcmVkLCBjdXJ0aHJlYWQpOworCWlmIChlcnJvciAh PSAwICYmICFleGNsdXNpdmUpCisJCWVycm9yID0gVk9QX0FDQ0VTUyh2cy0+dnNfdnAsIFZSRUFE LCBjcmVkLCBjdXJ0aHJlYWQpOwogCWlmIChlcnJvcikKIAkJZ290byBvdXQ7CiAKQEAgLTE4OTYs NyArMTg5OCw3IEBAIG5sbV9kb190ZXN0KG5sbTRfdGVzdGFyZ3MgKmFyZ3AsIG5sbTRfdGUKIAkJ Z290byBvdXQ7CiAJfQogCi0JZXJyb3IgPSBubG1fZ2V0X3Zmc19zdGF0ZShob3N0LCBycXN0cCwg JmZoLCAmdnMpOworCWVycm9yID0gbmxtX2dldF92ZnNfc3RhdGUoaG9zdCwgcnFzdHAsICZmaCwg JnZzLCBhcmdwLT5leGNsdXNpdmUpOwogCWlmIChlcnJvcikgewogCQlyZXN1bHQtPnN0YXQuc3Rh dCA9IG5sbV9jb252ZXJ0X2Vycm9yKGVycm9yKTsKIAkJZ290byBvdXQ7CkBAIC0yMDAxLDcgKzIw MDMsNyBAQCBubG1fZG9fbG9jayhubG00X2xvY2thcmdzICphcmdwLCBubG00X3JlCiAJCWdvdG8g b3V0OwogCX0KIAotCWVycm9yID0gbmxtX2dldF92ZnNfc3RhdGUoaG9zdCwgcnFzdHAsICZmaCwg JnZzKTsKKwllcnJvciA9IG5sbV9nZXRfdmZzX3N0YXRlKGhvc3QsIHJxc3RwLCAmZmgsICZ2cywg YXJncC0+ZXhjbHVzaXZlKTsKIAlpZiAoZXJyb3IpIHsKIAkJcmVzdWx0LT5zdGF0LnN0YXQgPSBu bG1fY29udmVydF9lcnJvcihlcnJvcik7CiAJCWdvdG8gb3V0OwpAQCAtMjE4MCw3ICsyMTgyLDcg QEAgbmxtX2RvX2NhbmNlbChubG00X2NhbmNhcmdzICphcmdwLCBubG00XwogCQlnb3RvIG91dDsK IAl9CiAKLQllcnJvciA9IG5sbV9nZXRfdmZzX3N0YXRlKGhvc3QsIHJxc3RwLCAmZmgsICZ2cyk7 CisJZXJyb3IgPSBubG1fZ2V0X3Zmc19zdGF0ZShob3N0LCBycXN0cCwgJmZoLCAmdnMsIGFyZ3At PmV4Y2x1c2l2ZSk7CiAJaWYgKGVycm9yKSB7CiAJCXJlc3VsdC0+c3RhdC5zdGF0ID0gbmxtX2Nv bnZlcnRfZXJyb3IoZXJyb3IpOwogCQlnb3RvIG91dDsKQEAgLTIyNjksNyArMjI3MSwxMSBAQCBu bG1fZG9fdW5sb2NrKG5sbTRfdW5sb2NrYXJncyAqYXJncCwgbmxtCiAJCWdvdG8gb3V0OwogCX0K IAotCWVycm9yID0gbmxtX2dldF92ZnNfc3RhdGUoaG9zdCwgcnFzdHAsICZmaCwgJnZzKTsKKwkv KgorCSAqIFNwZWNpZnkgRkFMU0Ugc28gdGhhdCBpdCB3aWxsIHRyeSB0aGUgY2FzZSB3aGVyZSB0 aGVyZSBpcworCSAqIFZSRUFEIGFjY2Vzcywgd2hlbiBWV1JJVEUgYWNjZXNzIGlzIGRlbmllZC4K KwkgKi8KKwllcnJvciA9IG5sbV9nZXRfdmZzX3N0YXRlKGhvc3QsIHJxc3RwLCAmZmgsICZ2cywg RkFMU0UpOwogCWlmIChlcnJvcikgewogCQlyZXN1bHQtPnN0YXQuc3RhdCA9IG5sbV9jb252ZXJ0 X2Vycm9yKGVycm9yKTsKIAkJZ290byBvdXQ7Cg== ------=_Part_953379_5444943.1323135388257--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1318843827.953380.1323135388258.JavaMail.root>