Re: [Bug 1531747] Re: overlay: mkdir fails if directory exists in lowerdir in a user namespace
> Something I'm still not sure about is what would happen if you made a > symlink, bind mount, etc. in upperdir with the same name as an unrelated > file in lowerdir. This is worth checking out. just tried a symlink and it didn't seem to affect the host directory (/opt/cisco) which was symlinked to /tmp/upper/cisco in the container to begin with. > > It looks like no, since > > > > root@w1:/tmp# mount -t overlay -o > > lowerdir=lower,upperdir=upper,workdir=workdir overlay /mnt > > root@w1:/tmp# ls /mnt > > cisco > > root@w1:/tmp# rmdir /mnt/cisco > > rmdir: failed to remove ‘/mnt/cisco’: Read-only file system > > root@w1:/tmp# mv /mnt/cisco /mnt/c2 > > mv: cannot move ‘/mnt/cisco’ to ‘/mnt/c2’: Read-only file system > > > > (here w1 is a unpriv container with /hostopt a bind mount of /opt on the > > host; cisco a directory both in host's /opt and in /tmp/lowerdir) > > I think I'm missing something here. I don't know why your mount is > read-only. Because a directory in workdir is owned by uid -1 (root on the host). > But even if it wasn't, cisco is in lowerdir and thus should > never be modified or removed in any case. Removing it in /mnt should (I Right, but I was trying to use workdir as a vector to make changes to something in the host's opt. Not lowerdir. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1531747 Title: overlay: mkdir fails if directory exists in lowerdir in a user namespace To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1531747/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
Re: [Bug 1531747] Re: overlay: mkdir fails if directory exists in lowerdir in a user namespace
On Thu, Jan 14, 2016 at 12:17:06AM -, Serge Hallyn wrote: > Does it require the workdir to be empty? Not empty. It wants to remove workdir/work if it exists and then create workdir/work, see ovl_workdir_create. Removing workdir/work is done as the user which is mounting, so nothing can be removed that the user couldn't already remove. ovl_workdir_create is ultimately just using vfs_rmdir if the directory exists, which is going to fail if workdir/work isn't empty. > I.e. is there a way (symlink, bind mount, something else) that a user > could use a dir they own which has a child which they don't own? I don't think so, since 1. lowerdir is not written to. 2. We've established that what's done with workdir won't make this possible. 3. When the vfs checks inode permissions for writing ovl_permission is called, which checks the permissions for the inode in upperdir for any write operation. I've checked most of the places where overlay fs raises its credentials, and I haven't found anything where it's doing anything with elevated creds that isn't in line with the permission checks that would have already been done. I'm still looking at ovl_rename2 though, which is doing a lot more stuff with elevated creds than anywhere else. Something I'm still not sure about is what would happen if you made a symlink, bind mount, etc. in upperdir with the same name as an unrelated file in lowerdir. This is worth checking out. > It looks like no, since > > root@w1:/tmp# mount -t overlay -o > lowerdir=lower,upperdir=upper,workdir=workdir overlay /mnt > root@w1:/tmp# ls /mnt > cisco > root@w1:/tmp# rmdir /mnt/cisco > rmdir: failed to remove ‘/mnt/cisco’: Read-only file system > root@w1:/tmp# mv /mnt/cisco /mnt/c2 > mv: cannot move ‘/mnt/cisco’ to ‘/mnt/c2’: Read-only file system > > (here w1 is a unpriv container with /hostopt a bind mount of /opt on the > host; cisco a directory both in host's /opt and in /tmp/lowerdir) I think I'm missing something here. I don't know why your mount is read-only. But even if it wasn't, cisco is in lowerdir and thus should never be modified or removed in any case. Removing it in /mnt should (I think, still learning here) result in making a file in upperdir with the whiteout xattr set. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1531747 Title: overlay: mkdir fails if directory exists in lowerdir in a user namespace To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1531747/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
Re: [Bug 1531747] Re: overlay: mkdir fails if directory exists in lowerdir in a user namespace
On Tue, Jan 12, 2016 at 05:16:28PM -, Serge Hallyn wrote: > I'm still looking, but it may be safe to say that all needed inode > checks are already done before we call ovl_set_opaque() so that we > can indeed just use prepare_kernel_cred(NULL) instead of prepare_cred(). I've been looking too, only at ovl_create_over_whiteout so far. Here's my take, though I'm still not an overlayfs expert so I may have missed something. Obviously the vfs is going to validate permissions to do the requested operation in the overlayfs superblock. This will result in ovl_permission getting called, which (for any write operation) is going to validate that the caller has the necessary permissions towards the relevant inode in uppderdir. That's good. So by the time we raise the caps and call ovl_create_over_whiteout we know that the user is allowed to do the operation, and ovl_create_or_link is only changing upperdir in the ways needed to satisfy this operation. Thus having the kernel assume full-on CAP_SYS_ADMIN before calling ovl_create_or_link seems fine wrt to upperdir. But nothing is being checked wrt workdir before elevating the capabilities. So maybe there are some checks at mount time to make sure the user has permissions needed towards workdir? ovl_fill_super doesn't appear to do so explicitly. However it does call ovl_workdir_create, and this expects that $WORKDIR/work doesn't already exist (where $WORKDIR is the string passed to mount via workdir=). If it does exist it tries to remove it with either vfs_rmdir or vfs_unlink (both of which call may_delete), then it tries to create $WORKDIR/work using vfs_mkdir (which calls may_create). If either of these fails the mount is read-only. This directory is what is used as the workdir for the mount, so I think this effectively validates that the kernel can safely modify the workdir in the same ways that it's modifying the upperdir. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1531747 Title: overlay: mkdir fails if directory exists in lowerdir in a user namespace To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1531747/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
Re: [Bug 1531747] Re: overlay: mkdir fails if directory exists in lowerdir in a user namespace
On Tue, Jan 12, 2016 at 04:56:24PM -, Serge Hallyn wrote: > Quoting Seth Forshee (seth.forshee...@canonical.com): > > I don't know why #2 is that much grosser than what's there now. It's > > I didn't mean gross as in eeuw, I meant not fine-grained enough. > > Because the capability will apply to inode permissions checks, > and we only want it to be used for the check authorizing the > writing of the trusted.overlay.opaque xattr. That makes more sense. And that's the part that concerns me the most too. > > already only taking the cap for setting the xattr, and taking > > CAP_SYS_ADMIN in init_user_ns seems to be what it's really wanting to do > > Maybe - that's what I'm not sure about. As you said earlier, in the > upstream code only an admin can do the actual mount. The fact that an > unpriv user can create the mount may change assumptions about the > underlying fs's. Yeah, that's something I'm just not sure about. It seems like by allowing the unprivileged user to mount in the first place we're implicitly saying that it's okay to write these xattrs to the underlying fs based on checks which happen at mount time. I don't know what checks are actually done at mount time though; unless we've augmented them they may be minimal based on the assumption that only CAP_SYS_ADMIN can mount. > > If we were to use ns_capable, which namespace do we use? > > I don't know. We're almost better off shipping a new version of > vfs_xattr() which is only for use by kernel writers. Maybe so. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1531747 Title: overlay: mkdir fails if directory exists in lowerdir in a user namespace To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1531747/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
Re: [Bug 1531747] Re: overlay: mkdir fails if directory exists in lowerdir in a user namespace
On Tue, Jan 12, 2016 at 05:16:28PM -, Serge Hallyn wrote: > in ovl_clear_empty(), the opaque bit is set on the dir in workingdir > > in ovl_create_over_whiteout() (the case we're currently looking at) it is > also being set in the working dir. > > in ovl_rename2(), it is set in two places, on the upper dentries for > both the old and new. > > So it is never set on the lowerdir, at least. Ah. So if it's never set on the lowerdir that does remove much of the concern. > I'm still looking, but it may be safe to say that all needed inode > checks are already done before we call ovl_set_opaque() so that we > can indeed just use prepare_kernel_cred(NULL) instead of prepare_cred(). Cool. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1531747 Title: overlay: mkdir fails if directory exists in lowerdir in a user namespace To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1531747/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
Re: [Bug 1531747] Re: overlay: mkdir fails if directory exists in lowerdir in a user namespace
Quoting Seth Forshee (seth.forshee...@canonical.com): > I don't know why #2 is that much grosser than what's there now. It's I didn't mean gross as in eeuw, I meant not fine-grained enough. Because the capability will apply to inode permissions checks, and we only want it to be used for the check authorizing the writing of the trusted.overlay.opaque xattr. > already only taking the cap for setting the xattr, and taking > CAP_SYS_ADMIN in init_user_ns seems to be what it's really wanting to do Maybe - that's what I'm not sure about. As you said earlier, in the upstream code only an admin can do the actual mount. The fact that an unpriv user can create the mount may change assumptions about the underlying fs's. > there. The difference now though is that before that capability would > have been required to do the mount and now it isn't. Right. > If we were to use ns_capable, which namespace do we use? I don't know. We're almost better off shipping a new version of vfs_xattr() which is only for use by kernel writers. If we had your patch we could maybe check against the sb->user_ns? > current_user_ns? Then that check becomes worthless because any user can > make a new namespace to bypass it. If we had the s_user_ns patches it Quit saying in the next paragraph what I say in reply to the previous! > might make sense to use that, but that probably doesn't solve the > problem anyway since the lower mount was probably mounted in > init_user_ns. Good point, hadn't thought of that. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1531747 Title: overlay: mkdir fails if directory exists in lowerdir in a user namespace To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1531747/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
Re: [Bug 1531747] Re: overlay: mkdir fails if directory exists in lowerdir in a user namespace
in ovl_clear_empty(), the opaque bit is set on the dir in workingdir in ovl_create_over_whiteout() (the case we're currently looking at) it is also being set in the working dir. in ovl_rename2(), it is set in two places, on the upper dentries for both the old and new. So it is never set on the lowerdir, at least. I'm still looking, but it may be safe to say that all needed inode checks are already done before we call ovl_set_opaque() so that we can indeed just use prepare_kernel_cred(NULL) instead of prepare_cred(). -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1531747 Title: overlay: mkdir fails if directory exists in lowerdir in a user namespace To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1531747/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
Re: [Bug 1531747] Re: overlay: mkdir fails if directory exists in lowerdir in a user namespace
Quoting Joseph Salisbury (joseph.salisb...@canonical.com): > Can you see if this bug also happens with the latest mainline kernel? It can > be downloaded from: That is not an option, because the mainline kernel doesn't support unprivileged overlayfs mounting which is where this happens. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1531747 Title: overlay: mkdir fails if directory exists in lowerdir in a user namespace To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1531747/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
