On Fri, Jun 01, 2018 at 09:14:38PM +0200, Miklos Szeredi wrote: > On Fri, Jun 1, 2018 at 7:43 PM, bfields@xxxxxxxxxxxx > <bfields@xxxxxxxxxxxx> wrote: > > On Fri, Jun 01, 2018 at 07:02:20PM +0200, Miklos Szeredi wrote: > >> On Fri, Jun 1, 2018 at 6:08 PM, bfields@xxxxxxxxxxxx > >> <bfields@xxxxxxxxxxxx> wrote: > >> > On Fri, Jun 01, 2018 at 04:43:51PM +0200, Miklos Szeredi wrote: > >> >> Look at ovl_permission(), I think it pretty clearly describes this model. > >> > > >> > Thanks! Uh, so generic_permission is the thing that just does the usual > >> > mode/acl checks on the in-core inode, and inode_permission is the one > >> > that also calls into the filesystem? > >> > >> Right. > >> > >> > But I'm still a little confused--if I'm reading right, "realinode" is > >> > the lower inode before copyup, and the upper inode after, so can't > >> > inode_permission(realinode, mask) return different results before and > >> > after copyup? > >> > >> Theoretically, yes. Not in any sane setup, though. > > > > If root squashing is enabled and you mount as root, then it will change. > > > > That's not an unlikely case, it's pretty much the default. > > Let's see: root squashing will change root creds to nobody's creds, right? > > That results in a weird situation, where user foo is trying to access > a file owned by foo, but which doesn't have read permission for > "other" will not be able to perform a read on that file. In fact, no > user will be able to read that file, including root. Copying up the > file will also fail, since that requires read permission. > > So for that case it's not true that permission will change, since > copy-up won't be possible. Fair enough. That's still pretty weird behavior. Arguably if you're going to do something different here then maybe it's good for the difference to manifest in a way that's really obvious in common cases, so people don't just overlook it. > That leaves the case where no read or write permissions are involved, > which is execute. I think we should just mask that out from the > inode_permission() check. Makes no sense to ask underlying filesystem > about execute permission. I suppose so. > With that we'll have consistent permissions before and after copy-up. > Right? Hm. If inode_permission(realinode, mask) changes across copy-up: - if the difference is in execute, you're telling me we'll ignore that. - if it changes from denying to allowing read, you don't care since that will prevent the copy up in the first place. - if it changes from denying to allowing write, that has no effect since you only care about read permission for copyup in the case someone requests write permission. (Hm, the special file case should be weird. I guess those just don't get copied up? Still I think people will be surprised by permission failures here.) - if it changes from allowing read or write to denying--then we're no longer talking about root, assuming the permission check on the copied-up inode wouldn't fail for root. We don't currently allow non-root to mount NFS, but there are people that would like to see that change. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html