| Message ID | 20180523232538.4880-2-ebiederm@xmission.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 23, 2018 at 06:25:34PM -0500, Eric W. Biederman wrote: > These filesystems already always set SB_I_NODEV so mknod will not be > useful for gaining control of any devices no matter their permissions. > This will allow overlayfs and applications to fakeroot to use device > nodes to represent things on disk. > > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> For a normal filesystem this does seem safe enough. However, I'd also like to see us allow unprivileged mounting for overlayfs, and there we need to worry about whether this would allow a mknod in an underlying filesystem which should not be allowed. That mknod will be subject to this same check in the underlying filesystem using the credentials of the user that mounted the overaly fs, which should be sufficient to ensure that the mknod is permitted. Thus this looks okay to me. Acked-by: Seth Forshee <seth.forshee@canonical.com>
Seth Forshee <seth.forshee@canonical.com> writes: > On Wed, May 23, 2018 at 06:25:34PM -0500, Eric W. Biederman wrote: >> These filesystems already always set SB_I_NODEV so mknod will not be >> useful for gaining control of any devices no matter their permissions. >> This will allow overlayfs and applications to fakeroot to use device >> nodes to represent things on disk. >> >> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > > For a normal filesystem this does seem safe enough. > > However, I'd also like to see us allow unprivileged mounting for > overlayfs, and there we need to worry about whether this would allow a > mknod in an underlying filesystem which should not be allowed. That > mknod will be subject to this same check in the underlying filesystem > using the credentials of the user that mounted the overaly fs, which > should be sufficient to ensure that the mknod is permitted. Sufficient to ensure the mknod is not permitted on the underlying filesystem. I believe you mean. > Thus this looks okay to me. > > Acked-by: Seth Forshee <seth.forshee@canonical.com> Eric
On Thu, May 24, 2018 at 11:55:45AM -0500, Eric W. Biederman wrote: > Seth Forshee <seth.forshee@canonical.com> writes: > > > On Wed, May 23, 2018 at 06:25:34PM -0500, Eric W. Biederman wrote: > >> These filesystems already always set SB_I_NODEV so mknod will not be > >> useful for gaining control of any devices no matter their permissions. > >> This will allow overlayfs and applications to fakeroot to use device > >> nodes to represent things on disk. > >> > >> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > > > > For a normal filesystem this does seem safe enough. > > > > However, I'd also like to see us allow unprivileged mounting for > > overlayfs, and there we need to worry about whether this would allow a > > mknod in an underlying filesystem which should not be allowed. That > > mknod will be subject to this same check in the underlying filesystem > > using the credentials of the user that mounted the overaly fs, which > > should be sufficient to ensure that the mknod is permitted. > > Sufficient to ensure the mknod is not permitted on the underlying > filesystem. I believe you mean. Right, or in other words with the relaxed capability check a user still could not use an overlayfs mount in a user namespace to mknod in a filesystem when that user couldn't otherwise mknod in that filesystem. Sorry if I wasn't clear. > > > Thus this looks okay to me. > > > > Acked-by: Seth Forshee <seth.forshee@canonical.com> > > Eric >
On Wed, May 23, 2018 at 06:25:34PM -0500, Eric W. Biederman wrote: > These filesystems already always set SB_I_NODEV so mknod will not be > useful for gaining control of any devices no matter their permissions. > This will allow overlayfs and applications to fakeroot to use device > nodes to represent things on disk. Excellent. Acked-by: Christian Brauner <christian@brauner.io> > > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > fs/namei.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/namei.c b/fs/namei.c > index 942c1f096f6b..20335896dcce 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -3679,7 +3679,8 @@ int vfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, dev_t dev) > if (error) > return error; > > - if ((S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD)) > + if ((S_ISCHR(mode) || S_ISBLK(mode)) && > + !ns_capable(dentry->d_sb->s_user_ns, CAP_MKNOD)) > return -EPERM; > > if (!dir->i_op->mknod) > -- > 2.14.1 >
diff --git a/fs/namei.c b/fs/namei.c index 942c1f096f6b..20335896dcce 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3679,7 +3679,8 @@ int vfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, dev_t dev) if (error) return error; - if ((S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD)) + if ((S_ISCHR(mode) || S_ISBLK(mode)) && + !ns_capable(dentry->d_sb->s_user_ns, CAP_MKNOD)) return -EPERM; if (!dir->i_op->mknod)
These filesystems already always set SB_I_NODEV so mknod will not be useful for gaining control of any devices no matter their permissions. This will allow overlayfs and applications to fakeroot to use device nodes to represent things on disk. Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- fs/namei.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)