[PATCH 0/1][B] CVE-2018-6559 - Filename information disclosure in overlayfs

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/1][B] CVE-2018-6559 - Filename information disclosure in overlayfs

Tyler Hicks-2
https://people.canonical.com/~ubuntu-security/cve/2018/CVE-2018-6559.html
https://launchpad.net/bugs/1793458/

 The overlayfs implementation in the linux (aka Linux kernel) package in Ubuntu
 did not properly check permissions for read operations on directories in the
 lower filesystem directory, which allows local users to obtain names of files
 in which they would not normally be able to access by performing an overlayfs
 mount inside of a user namespace.

I've tested this change with a QRT regression test that I wrote as well as the
unionmount-testsuite:

https://github.com/amir73il/unionmount-testsuite.git

This issue is related to a portion of CVE-2015-1328 that was reintroduced into
the Ubuntu kernel. This bug comment describes the situation:

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1793458/comments/4

As mentioned above, I wrote a QRT test for this issue so that we don't
accidentally drop our SAUCE patch in the future.

Tyler


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1][B] UBUNTU: SAUCE: overlayfs: ensure mounter privileges when reading directories

Tyler Hicks-2
From: Andy Whitcroft <[hidden email]>

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 <[hidden email]>
[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 <[hidden email]>
---
 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 0dbbfedef54c..21905e30f50d 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -171,7 +171,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 */
@@ -188,15 +187,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 b1917c414e93..7b06983a8447 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -204,6 +204,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);
 bool ovl_can_decode_fh(struct super_block *sb);
 struct dentry *ovl_indexdir(struct super_block *sb);
 struct ovl_entry *ovl_alloc_entry(unsigned int numlower);
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index e258c234f357..fa9469325334 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -369,6 +369,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)
@@ -688,6 +694,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 06119f34a69d..ea2c28fd7be5 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -50,6 +50,19 @@ struct super_block *ovl_same_sb(struct super_block *sb)
  return ofs->same_sb;
 }
 
+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;
+}
+
 bool ovl_can_decode_fh(struct super_block *sb)
 {
  return (sb->s_export_op && sb->s_export_op->fh_to_dentry &&
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 1/1][B] UBUNTU: SAUCE: overlayfs: ensure mounter privileges when reading directories

Seth Forshee
On Fri, Oct 19, 2018 at 04:45:39PM +0000, Tyler Hicks wrote:

> From: Andy Whitcroft <[hidden email]>
>
> 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 <[hidden email]>
> [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 <[hidden email]>

Acked-by: Seth Forshee <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [PATCH 1/1][B] UBUNTU: SAUCE: overlayfs: ensure mounter privileges when reading directories

Stefan Bader-2
In reply to this post by Tyler Hicks-2
On 19.10.18 18:45, Tyler Hicks wrote:

> From: Andy Whitcroft <[hidden email]>
>
> 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 <[hidden email]>
> [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 <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>
> ---

Same as for Cosmic version.

>  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 0dbbfedef54c..21905e30f50d 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -171,7 +171,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 */
> @@ -188,15 +187,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 b1917c414e93..7b06983a8447 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -204,6 +204,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);
>  bool ovl_can_decode_fh(struct super_block *sb);
>  struct dentry *ovl_indexdir(struct super_block *sb);
>  struct ovl_entry *ovl_alloc_entry(unsigned int numlower);
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index e258c234f357..fa9469325334 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -369,6 +369,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)
> @@ -688,6 +694,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 06119f34a69d..ea2c28fd7be5 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -50,6 +50,19 @@ struct super_block *ovl_same_sb(struct super_block *sb)
>   return ofs->same_sb;
>  }
>  
> +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;
> +}
> +
>  bool ovl_can_decode_fh(struct super_block *sb)
>  {
>   return (sb->s_export_op && sb->s_export_op->fh_to_dentry &&
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED: [PATCH 0/1][B] CVE-2018-6559 - Filename information disclosure in overlayfs

Stefan Bader-2
In reply to this post by Tyler Hicks-2
On 19.10.18 18:45, Tyler Hicks wrote:

> https://people.canonical.com/~ubuntu-security/cve/2018/CVE-2018-6559.html
> https://launchpad.net/bugs/1793458/
>
>  The overlayfs implementation in the linux (aka Linux kernel) package in Ubuntu
>  did not properly check permissions for read operations on directories in the
>  lower filesystem directory, which allows local users to obtain names of files
>  in which they would not normally be able to access by performing an overlayfs
>  mount inside of a user namespace.
>
> I've tested this change with a QRT regression test that I wrote as well as the
> unionmount-testsuite:
>
> https://github.com/amir73il/unionmount-testsuite.git
>
> This issue is related to a portion of CVE-2015-1328 that was reintroduced into
> the Ubuntu kernel. This bug comment describes the situation:
>
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1793458/comments/4
>
> As mentioned above, I wrote a QRT test for this issue so that we don't
> accidentally drop our SAUCE patch in the future.
>
> Tyler
>
>
Applied to bionic/master-next. Thanks.

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment