Message ID | 1539967493-18941-2-git-send-email-tyhicks@canonical.com |
---|---|
State | New |
Headers | show |
Series | CVE-2018-6559 - Filename information disclosure in overlayfs | expand |
On Fri, Oct 19, 2018 at 04:44:53PM +0000, Tyler Hicks wrote: > From: Andy Whitcroft <apw@canonical.com> > > BugLink: https://launchpad.net/bugs/1793458 > > When reading directory contents ensure the mounter has permissions for > the operation over the constituent parts (lower and upper). Where we are > in a namespace this ensures that the mounter (root in that namespace) > has permissions over the files and directories, preventing exposure of > protected files and directory contents. > > CVE-2018-6559 > > Signed-off-by: Andy Whitcroft <apw@canonical.com> > [tyhicks: make use of new upstream check in ovl_permission() for copy-ups] > [tyhicks: make use of creator (mounter) creds hanging off the super block] > Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Acked-by: Seth Forshee <seth.forshee@canonical.com> Applied to unstable/master, thanks!
On 19.10.18 18:44, Tyler Hicks wrote: > From: Andy Whitcroft <apw@canonical.com> > > BugLink: https://launchpad.net/bugs/1793458 > > When reading directory contents ensure the mounter has permissions for > the operation over the constituent parts (lower and upper). Where we are > in a namespace this ensures that the mounter (root in that namespace) > has permissions over the files and directories, preventing exposure of > protected files and directory contents. > > CVE-2018-6559 > > Signed-off-by: Andy Whitcroft <apw@canonical.com> > [tyhicks: make use of new upstream check in ovl_permission() for copy-ups] > [tyhicks: make use of creator (mounter) creds hanging off the super block] > Signed-off-by: Tyler Hicks <tyhicks@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- From the bug report I take this was taken from Xenial and forward ported to Bionic and Cosmic. I wonder whether we could make this more obvious in the s-o-b area something like (fwd-ported from commit <sha1> xenial) -Stefan > fs/overlayfs/inode.c | 5 +---- > fs/overlayfs/overlayfs.h | 2 ++ > fs/overlayfs/readdir.c | 12 ++++++++++++ > fs/overlayfs/util.c | 13 +++++++++++++ > 4 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c > index ed16a898caeb..988f7bdde3de 100644 > --- a/fs/overlayfs/inode.c > +++ b/fs/overlayfs/inode.c > @@ -217,7 +217,6 @@ int ovl_permission(struct inode *inode, int mask) > { > struct inode *upperinode = ovl_inode_upper(inode); > struct inode *realinode = upperinode ?: ovl_inode_lower(inode); > - const struct cred *old_cred; > int err; > > /* Careful in RCU walk mode */ > @@ -234,15 +233,13 @@ int ovl_permission(struct inode *inode, int mask) > if (err) > return err; > > - old_cred = ovl_override_creds(inode->i_sb); > if (!upperinode && > !special_file(realinode->i_mode) && mask & MAY_WRITE) { > mask &= ~(MAY_WRITE | MAY_APPEND); > /* Make sure mounter can read file for copy up later */ > mask |= MAY_READ; > } > - err = inode_permission(realinode, mask); > - revert_creds(old_cred); > + err = ovl_creator_permission(inode->i_sb, realinode, mask); > > return err; > } > diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h > index 31aebb429d02..62c3c080b185 100644 > --- a/fs/overlayfs/overlayfs.h > +++ b/fs/overlayfs/overlayfs.h > @@ -208,6 +208,8 @@ void ovl_drop_write(struct dentry *dentry); > struct dentry *ovl_workdir(struct dentry *dentry); > const struct cred *ovl_override_creds(struct super_block *sb); > struct super_block *ovl_same_sb(struct super_block *sb); > +int ovl_creator_permission(struct super_block *sb, struct inode *inode, > + int mode); > int ovl_can_decode_fh(struct super_block *sb); > struct dentry *ovl_indexdir(struct super_block *sb); > bool ovl_index_all(struct super_block *sb); > diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c > index cc8303a806b4..d8ec95a4069f 100644 > --- a/fs/overlayfs/readdir.c > +++ b/fs/overlayfs/readdir.c > @@ -373,6 +373,12 @@ static int ovl_dir_read_merged(struct dentry *dentry, struct list_head *list, > next = ovl_path_next(idx, dentry, &realpath); > rdd.is_upper = ovl_dentry_upper(dentry) == realpath.dentry; > > + err = ovl_creator_permission(dentry->d_sb, > + d_inode(realpath.dentry), > + MAY_READ); > + if (err) > + break; > + > if (next != -1) { > err = ovl_dir_read(&realpath, &rdd); > if (err) > @@ -735,6 +741,12 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx) > ovl_dir_reset(file); > > if (od->is_real) { > + err = ovl_creator_permission(dentry->d_sb, > + file_inode(od->realfile), > + MAY_READ); > + if (err) > + return err; > + > /* > * If parent is merge, then need to adjust d_ino for '..', if > * dir is impure then need to adjust d_ino for copied up > diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c > index 6f1078028c66..2ca86dbb55b6 100644 > --- a/fs/overlayfs/util.c > +++ b/fs/overlayfs/util.c > @@ -55,6 +55,19 @@ struct super_block *ovl_same_sb(struct super_block *sb) > return NULL; > } > > +int ovl_creator_permission(struct super_block *sb, struct inode *inode, > + int mode) > +{ > + const struct cred *old_cred; > + int err = 0; > + > + old_cred = ovl_override_creds(sb); > + err = inode_permission(inode, mode); > + revert_creds(old_cred); > + > + return err; > +} > + > /* > * Check if underlying fs supports file handles and try to determine encoding > * type, in order to deduce maximum inode number used by fs. >
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index ed16a898caeb..988f7bdde3de 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -217,7 +217,6 @@ int ovl_permission(struct inode *inode, int mask) { struct inode *upperinode = ovl_inode_upper(inode); struct inode *realinode = upperinode ?: ovl_inode_lower(inode); - const struct cred *old_cred; int err; /* Careful in RCU walk mode */ @@ -234,15 +233,13 @@ int ovl_permission(struct inode *inode, int mask) if (err) return err; - old_cred = ovl_override_creds(inode->i_sb); if (!upperinode && !special_file(realinode->i_mode) && mask & MAY_WRITE) { mask &= ~(MAY_WRITE | MAY_APPEND); /* Make sure mounter can read file for copy up later */ mask |= MAY_READ; } - err = inode_permission(realinode, mask); - revert_creds(old_cred); + err = ovl_creator_permission(inode->i_sb, realinode, mask); return err; } diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 31aebb429d02..62c3c080b185 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -208,6 +208,8 @@ void ovl_drop_write(struct dentry *dentry); struct dentry *ovl_workdir(struct dentry *dentry); const struct cred *ovl_override_creds(struct super_block *sb); struct super_block *ovl_same_sb(struct super_block *sb); +int ovl_creator_permission(struct super_block *sb, struct inode *inode, + int mode); int ovl_can_decode_fh(struct super_block *sb); struct dentry *ovl_indexdir(struct super_block *sb); bool ovl_index_all(struct super_block *sb); diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index cc8303a806b4..d8ec95a4069f 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -373,6 +373,12 @@ static int ovl_dir_read_merged(struct dentry *dentry, struct list_head *list, next = ovl_path_next(idx, dentry, &realpath); rdd.is_upper = ovl_dentry_upper(dentry) == realpath.dentry; + err = ovl_creator_permission(dentry->d_sb, + d_inode(realpath.dentry), + MAY_READ); + if (err) + break; + if (next != -1) { err = ovl_dir_read(&realpath, &rdd); if (err) @@ -735,6 +741,12 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx) ovl_dir_reset(file); if (od->is_real) { + err = ovl_creator_permission(dentry->d_sb, + file_inode(od->realfile), + MAY_READ); + if (err) + return err; + /* * If parent is merge, then need to adjust d_ino for '..', if * dir is impure then need to adjust d_ino for copied up diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 6f1078028c66..2ca86dbb55b6 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -55,6 +55,19 @@ struct super_block *ovl_same_sb(struct super_block *sb) return NULL; } +int ovl_creator_permission(struct super_block *sb, struct inode *inode, + int mode) +{ + const struct cred *old_cred; + int err = 0; + + old_cred = ovl_override_creds(sb); + err = inode_permission(inode, mode); + revert_creds(old_cred); + + return err; +} + /* * Check if underlying fs supports file handles and try to determine encoding * type, in order to deduce maximum inode number used by fs.