Skip to content
Snippets Groups Projects
  1. May 21, 2019
  2. May 08, 2019
    • Amir Goldstein's avatar
      ovl: relax WARN_ON() for overlapping layers use case · acf3062a
      Amir Goldstein authored
      This nasty little syzbot repro:
      https://syzkaller.appspot.com/x/repro.syz?x=12c7a94f400000
      
      
      
      Creates overlay mounts where the same directory is both in upper and lower
      layers. Simplified example:
      
        mkdir foo work
        mount -t overlay none foo -o"lowerdir=.,upperdir=foo,workdir=work"
      
      The repro runs several threads in parallel that attempt to chdir into foo
      and attempt to symlink/rename/exec/mkdir the file bar.
      
      The repro hits a WARN_ON() I placed in ovl_instantiate(), which suggests
      that an overlay inode already exists in cache and is hashed by the pointer
      of the real upper dentry that ovl_create_real() has just created. At the
      point of the WARN_ON(), for overlay dir inode lock is held and upper dir
      inode lock, so at first, I did not see how this was possible.
      
      On a closer look, I see that after ovl_create_real(), because of the
      overlapping upper and lower layers, a lookup by another thread can find the
      file foo/bar that was just created in upper layer, at overlay path
      foo/foo/bar and hash the an overlay inode with the new real dentry as lower
      dentry. This is possible because the overlay directory foo/foo is not
      locked and the upper dentry foo/bar is in dcache, so ovl_lookup() can find
      it without taking upper dir inode shared lock.
      
      Overlapping layers is considered a wrong setup which would result in
      unexpected behavior, but it shouldn't crash the kernel and it shouldn't
      trigger WARN_ON() either, so relax this WARN_ON() and leave a pr_warn()
      instead to cover all cases of failure to get an overlay inode.
      
      The error returned from failure to insert new inode to cache with
      inode_insert5() was changed to -EEXIST, to distinguish from the error
      -ENOMEM returned on failure to get/allocate inode with iget5_locked().
      
      Reported-by: default avatar <syzbot+9c69c282adc4edd2b540@syzkaller.appspotmail.com>
      Fixes: 01b39dcc ("ovl: use inode_insert5() to hash a newly...")
      Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      acf3062a
  3. May 06, 2019
  4. May 02, 2019
  5. Apr 26, 2019
  6. Feb 13, 2019
  7. Feb 04, 2019
    • Vivek Goyal's avatar
      ovl: During copy up, first copy up data and then xattrs · 5f32879e
      Vivek Goyal authored
      If a file with capability set (and hence security.capability xattr) is
      written kernel clears security.capability xattr. For overlay, during file
      copy up if xattrs are copied up first and then data is, copied up. This
      means data copy up will result in clearing of security.capability xattr
      file on lower has. And this can result into surprises. If a lower file has
      CAP_SETUID, then it should not be cleared over copy up (if nothing was
      actually written to file).
      
      This also creates problems with chown logic where it first copies up file
      and then tries to clear setuid bit. But by that time security.capability
      xattr is already gone (due to data copy up), and caller gets -ENODATA.
      This has been reported by Giuseppe here.
      
      https://github.com/containers/libpod/issues/2015#issuecomment-447824842
      
      
      
      Fix this by copying up data first and then metadta. This is a regression
      which has been introduced by my commit as part of metadata only copy up
      patches.
      
      TODO: There will be some corner cases where a file is copied up metadata
      only and later data copy up happens and that will clear security.capability
      xattr. Something needs to be done about that too.
      
      Fixes: bd64e575 ("ovl: During copy up, first copy up metadata and then data")
      Cc: <stable@vger.kernel.org> # v4.19+
      Reported-by: default avatarGiuseppe Scrivano <gscrivan@redhat.com>
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      5f32879e
  8. Dec 04, 2018
  9. Nov 21, 2018
    • Amir Goldstein's avatar
      ovl: fix decode of dir file handle with multi lower layers · 155b8a04
      Amir Goldstein authored
      
      When decoding a lower file handle, we first call ovl_check_origin_fh()
      with connected=false to get any real lower dentry for overlay inode
      cache lookup.
      
      If the real dentry is a disconnected dir dentry, ovl_check_origin_fh()
      is called again with connected=true to get a connected real dentry
      and find the lower layer the real dentry belongs to.
      
      If the first call returned a connected real dentry, we use it to
      lookup an overlay connected dentry, but the first ovl_check_origin_fh()
      call with connected=false did not check that the found dentry is under
      the root of the layer (see ovl_acceptable()), it only checked that
      the found dentry super block matches the uuid of the lower file handle.
      
      In case there are multiple lower layers on the same fs and the found
      dentry is not from the top most lower layer, using the layer index
      returned from the first ovl_check_origin_fh() is wrong and we end
      up failing to decode the file handle.
      
      Fix this by always calling ovl_check_origin_fh() with connected=true
      if we got a directory dentry in the first call.
      
      Fixes: 8b58924a ("ovl: lookup in inode cache first when decoding...")
      Cc: <stable@vger.kernel.org> # v4.17
      Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      155b8a04
  10. Nov 19, 2018
  11. Nov 01, 2018
    • Miklos Szeredi's avatar
      ovl: automatically enable redirect_dir on metacopy=on · d47748e5
      Miklos Szeredi authored
      
      Current behavior is to automatically disable metacopy if redirect_dir is
      not enabled and proceed with the mount.
      
      If "metacopy=on" mount option was given, then this behavior can confuse the
      user: no mount failure, yet metacopy is disabled.
      
      This patch makes metacopy=on imply redirect_dir=on.
      
      The converse is also true: turning off full redirect with redirect_dir=
      {off|follow|nofollow} will disable metacopy.
      
      If both metacopy=on and redirect_dir={off|follow|nofollow} is specified,
      then mount will fail, since there's no way to correctly resolve the
      conflict.
      
      Reported-by: default avatarDaniel Walsh <dwalsh@redhat.com>
      Fixes: d5791044 ("ovl: Provide a mount option metacopy=on/off...")
      Cc: <stable@vger.kernel.org> # v4.19
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      d47748e5
  12. Oct 31, 2018
    • Miklos Szeredi's avatar
      ovl: check whiteout in ovl_create_over_whiteout() · 5e127580
      Miklos Szeredi authored
      
      Kaixuxia repors that it's possible to crash overlayfs by removing the
      whiteout on the upper layer before creating a directory over it.  This is a
      reproducer:
      
       mkdir lower upper work merge
       touch lower/file
       mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work merge
       rm merge/file
       ls -al merge/file
       rm upper/file
       ls -al merge/
       mkdir merge/file
      
      Before commencing with a vfs_rename(..., RENAME_EXCHANGE) verify that the
      lookup of "upper" is positive and is a whiteout, and return ESTALE
      otherwise.
      
      Reported by: kaixuxia <xiakaixu1987@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      Fixes: e9be9d5e ("overlay filesystem")
      Cc: <stable@vger.kernel.org> # v3.18
      5e127580
  13. Oct 29, 2018
  14. Oct 26, 2018
  15. Oct 04, 2018
  16. Sep 25, 2018
  17. Sep 24, 2018
  18. Sep 10, 2018
  19. Sep 03, 2018
  20. Aug 30, 2018
    • Amir Goldstein's avatar
      ovl: fix GPF in swapfile_activate of file from overlayfs over xfs · 5b910bd6
      Amir Goldstein authored
      
      Since overlayfs implements stacked file operations, the underlying
      filesystems are not supposed to be exposed to the overlayfs file,
      whose f_inode is an overlayfs inode.
      
      Assigning an overlayfs file to swap_file results in an attempt of xfs
      code to dereference an xfs_inode struct from an ovl_inode pointer:
      
       CPU: 0 PID: 2462 Comm: swapon Not tainted
       4.18.0-xfstests-12721-g33e17876ea4e #3402
       RIP: 0010:xfs_find_bdev_for_inode+0x23/0x2f
       Call Trace:
        xfs_iomap_swapfile_activate+0x1f/0x43
        __se_sys_swapon+0xb1a/0xee9
      
      Fix this by not assigning the real inode mapping to f_mapping, which
      will cause swapon() to return an error (-EINVAL). Although it makes
      sense not to allow setting swpafile on an overlayfs file, some users
      may depend on it, so we may need to fix this up in the future.
      
      Keeping f_mapping pointing to overlay inode mapping will cause O_DIRECT
      open to fail. Fix this by installing ovl_aops with noop_direct_IO in
      overlay inode mapping.
      
      Keeping f_mapping pointing to overlay inode mapping will cause other
      a_ops related operations to fail (e.g. readahead()). Those will be
      fixed by follow up patches.
      
      Suggested-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      Fixes: f7c72396 ("ovl: add O_DIRECT support")
      Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      5b910bd6
    • Amir Goldstein's avatar
      ovl: respect FIEMAP_FLAG_SYNC flag · 80d34810
      Amir Goldstein authored
      
      Stacked overlayfs fiemap operation broke xfstests that test delayed
      allocation (with "_test_generic_punch -d"), because ovl_fiemap()
      failed to write dirty pages when requested.
      
      Fixes: 9e142c41 ("ovl: add ovl_fiemap()")
      Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      80d34810
Loading