On Sat, Jun 2, 2018 at 2:50 AM, bfields@xxxxxxxxxxxx <bfields@xxxxxxxxxxxx> wrote: > 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.) Special file can get copied up on metadata change. We shoudln't ask server at all for permissions in this case. That would be consistent with the "cp -a" model. > - 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. The "cp -a" model should work even for userns unprivileged mounts. We do assume a certain sanity with setups: I'm sure someone could come up with a setup (e.g. with selinux) where weirdness happens after copy-up. Thanks, Miklos > > --b. > -- > To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html