releases/4.19.31/ovl-during-copy-up-first-copy-up-data-and-then-xattrs.patch

   1 From 5f32879ea35523b9842bdbdc0065e13635caada2 Mon Sep 17 00:00:00 2001
   2 From: Vivek Goyal <vgoyal@redhat.com>
   3 Date: Fri, 11 Jan 2019 19:37:00 +0100
   4 Subject: ovl: During copy up, first copy up data and then xattrs
   5
   6 From: Vivek Goyal <vgoyal@redhat.com>
   7
   8 commit 5f32879ea35523b9842bdbdc0065e13635caada2 upstream.
   9
  10 If a file with capability set (and hence security.capability xattr) is
  11 written kernel clears security.capability xattr. For overlay, during file
  12 copy up if xattrs are copied up first and then data is, copied up. This
  13 means data copy up will result in clearing of security.capability xattr
  14 file on lower has. And this can result into surprises. If a lower file has
  15 CAP_SETUID, then it should not be cleared over copy up (if nothing was
  16 actually written to file).
  17
  18 This also creates problems with chown logic where it first copies up file
  19 and then tries to clear setuid bit. But by that time security.capability
  20 xattr is already gone (due to data copy up), and caller gets -ENODATA.
  21 This has been reported by Giuseppe here.
  22
  23 https://github.com/containers/libpod/issues/2015#issuecomment-447824842
  24
  25 Fix this by copying up data first and then metadta. This is a regression
  26 which has been introduced by my commit as part of metadata only copy up
  27 patches.
  28
  29 TODO: There will be some corner cases where a file is copied up metadata
  30 only and later data copy up happens and that will clear security.capability
  31 xattr. Something needs to be done about that too.
  32
  33 Fixes: bd64e57586d3 ("ovl: During copy up, first copy up metadata and then data")
  34 Cc: <stable@vger.kernel.org> # v4.19+
  35 Reported-by: Giuseppe Scrivano <gscrivan@redhat.com>
  36 Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
  37 Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
  38 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  39
  40 ---
  41  fs/overlayfs/copy_up.c |   31 ++++++++++++++++++-------------
  42  1 file changed, 18 insertions(+), 13 deletions(-)
  43
  44 --- a/fs/overlayfs/copy_up.c
  45 +++ b/fs/overlayfs/copy_up.c
  46 @@ -501,6 +501,24 @@ static int ovl_copy_up_inode(struct ovl_
  47  {
  48         int err;
  49
  50 +       /*
  51 +        * Copy up data first and then xattrs. Writing data after
  52 +        * xattrs will remove security.capability xattr automatically.
  53 +        */
  54 +       if (S_ISREG(c->stat.mode) && !c->metacopy) {
  55 +               struct path upperpath, datapath;
  56 +
  57 +               ovl_path_upper(c->dentry, &upperpath);
  58 +               if (WARN_ON(upperpath.dentry != NULL))
  59 +                       return -EIO;
  60 +               upperpath.dentry = temp;
  61 +
  62 +               ovl_path_lowerdata(c->dentry, &datapath);
  63 +               err = ovl_copy_up_data(&datapath, &upperpath, c->stat.size);
  64 +               if (err)
  65 +                       return err;
  66 +       }
  67 +
  68         err = ovl_copy_xattr(c->lowerpath.dentry, temp);
  69         if (err)
  70                 return err;
  71 @@ -517,19 +535,6 @@ static int ovl_copy_up_inode(struct ovl_
  72                 if (err)
  73                         return err;
  74         }
  75 -
  76 -       if (S_ISREG(c->stat.mode) && !c->metacopy) {
  77 -               struct path upperpath, datapath;
  78 -
  79 -               ovl_path_upper(c->dentry, &upperpath);
  80 -               BUG_ON(upperpath.dentry != NULL);
  81 -               upperpath.dentry = temp;
  82 -
  83 -               ovl_path_lowerdata(c->dentry, &datapath);
  84 -               err = ovl_copy_up_data(&datapath, &upperpath, c->stat.size);
  85 -               if (err)
  86 -                       return err;
  87 -       }
  88
  89         if (c->metacopy) {
  90                 err = ovl_check_setxattr(c->dentry, temp, OVL_XATTR_METACOPY,