Skip to content
Snippets Groups Projects
  1. Sep 02, 2020
  2. Jun 04, 2020
  3. Jun 02, 2020
    • Miklos Szeredi's avatar
      ovl: verify permissions in ovl_path_open() · 56230d95
      Miklos Szeredi authored
      
      Check permission before opening a real file.
      
      ovl_path_open() is used by readdir and copy-up routines.
      
      ovl_permission() theoretically already checked copy up permissions, but it
      doesn't hurt to re-do these checks during the actual copy-up.
      
      For directory reading ovl_permission() only checks access to topmost
      underlying layer.  Readdir on a merged directory accesses layers below the
      topmost one as well.  Permission wasn't checked for these layers.
      
      Note: modifying ovl_permission() to perform this check would be far more
      complex and hence more bug prone.  The result is less precise permissions
      returned in access(2).  If this turns out to be an issue, we can revisit
      this bug.
      
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      56230d95
  4. May 13, 2020
  5. Mar 17, 2020
  6. Mar 12, 2020
  7. Jan 24, 2020
  8. Jan 22, 2020
  9. Jun 19, 2019
  10. May 29, 2019
    • Amir Goldstein's avatar
      ovl: detect overlapping layers · 146d62e5
      Amir Goldstein authored
      Overlapping overlay layers are not supported and can cause unexpected
      behavior, but overlayfs does not currently check or warn about these
      configurations.
      
      User is not supposed to specify the same directory for upper and
      lower dirs or for different lower layers and user is not supposed to
      specify directories that are descendants of each other for overlay
      layers, but that is exactly what this zysbot repro did:
      
          https://syzkaller.appspot.com/x/repro.syz?x=12c7a94f400000
      
      
      
      Moving layer root directories into other layers while overlayfs
      is mounted could also result in unexpected behavior.
      
      This commit places "traps" in the overlay inode hash table.
      Those traps are dummy overlay inodes that are hashed by the layers
      root inodes.
      
      On mount, the hash table trap entries are used to verify that overlay
      layers are not overlapping.  While at it, we also verify that overlay
      layers are not overlapping with directories "in-use" by other overlay
      instances as upperdir/workdir.
      
      On lookup, the trap entries are used to verify that overlay layers
      root inodes have not been moved into other layers after mount.
      
      Some examples:
      
      $ ./run --ov --samefs -s
      ...
      ( mkdir -p base/upper/0/u base/upper/0/w base/lower lower upper mnt
        mount -o bind base/lower lower
        mount -o bind base/upper upper
        mount -t overlay none mnt ...
              -o lowerdir=lower,upperdir=upper/0/u,workdir=upper/0/w)
      
      $ umount mnt
      $ mount -t overlay none mnt ...
              -o lowerdir=base,upperdir=upper/0/u,workdir=upper/0/w
      
        [   94.434900] overlayfs: overlapping upperdir path
        mount: mount overlay on mnt failed: Too many levels of symbolic links
      
      $ mount -t overlay none mnt ...
              -o lowerdir=upper/0/u,upperdir=upper/0/u,workdir=upper/0/w
      
        [  151.350132] overlayfs: conflicting lowerdir path
        mount: none is already mounted or mnt busy
      
      $ mount -t overlay none mnt ...
              -o lowerdir=lower:lower/a,upperdir=upper/0/u,workdir=upper/0/w
      
        [  201.205045] overlayfs: overlapping lowerdir path
        mount: mount overlay on mnt failed: Too many levels of symbolic links
      
      $ mount -t overlay none mnt ...
              -o lowerdir=lower,upperdir=upper/0/u,workdir=upper/0/w
      $ mv base/upper/0/ base/lower/
      $ find mnt/0
        mnt/0
        mnt/0/w
        find: 'mnt/0/w/work': Too many levels of symbolic links
        find: 'mnt/0/u': Too many levels of symbolic links
      
      Reported-by: default avatar <syzbot+9c69c282adc4edd2b540@syzkaller.appspotmail.com>
      Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      146d62e5
  11. Feb 13, 2019
  12. Oct 26, 2018
  13. Sep 24, 2018
  14. Jul 20, 2018
    • Vivek Goyal's avatar
      ovl: Check redirect on index as well · 0a2d0d3f
      Vivek Goyal authored
      
      Right now we seem to check redirect only if upperdentry is found.  But it
      is possible that there is no upperdentry but later we found an index.
      
      We need to check redirect on index as well and set it in
      ovl_inode->redirect.  Otherwise link code can assume that dentry does not
      have redirect and place a new one which breaks things.  In my testing
      overlay/033 test started failing in xfstests.  Following are the details.
      
      For example do following.
      
      $ mkdir lower upper work merged
      
       - Make lower dir with 4 links.
        $ echo "foo" > lower/l0.txt
        $ ln  lower/l0.txt lower/l1.txt
        $ ln  lower/l0.txt lower/l2.txt
        $ ln  lower/l0.txt lower/l3.txt
      
       - Mount with index on and metacopy on.
      
        $ mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=work,\
                              index=on,metacopy=on none merged
      
       - Link lower
      
        $ ln merged/l0.txt merged/l4.txt
          (This will metadata copy up of l0.txt and put an absolute redirect
           /l0.txt)
      
        $ echo 2 > /proc/sys/vm/drop/caches
      
        $ ls merged/l1.txt
        (Now l1.txt will be looked up.  There is no upper dentry but there is
         lower dentry and index will be found.  We don't check for redirect on
         index, hence ovl_inode->redirect will be NULL.)
      
       - Link Upper
      
        $ ln merged/l4.txt merged/l5.txt
        (Lookup of l4.txt will use inode from l1.txt lookup which is still in
         cache.  It has ovl_inode->redirect NULL, hence link will put a new
         redirect and replace /l0.txt with /l4.txt
      
       - Drop caches.
        echo 2 > /proc/sys/vm/drop_caches
      
       - List l1.txt and it returns -ESTALE
      
        $ ls merged/l0.txt
      
        (It returns stale because, we found a metacopy of l0.txt in upper and it
         has redirect l4.txt but there is no file named l4.txt in lower layer.
         So lower data copy is not found and -ESTALE is returned.)
      
      So problem here is that we did not process redirect on index.  Check
      redirect on index as well and then problem is fixed.
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      0a2d0d3f
    • Vivek Goyal's avatar
      ovl: Do not set dentry type ORIGIN for broken hardlinks · 60124877
      Vivek Goyal authored
      
      If a dentry has copy up origin, we set flag OVL_PATH_ORIGIN.  So far this
      decision was easy that we had to check only for oe->numlower and if it is
      non-zero, we knew there is copy up origin.  (For non-dir we installed
      origin dentry in lowerstack[0]).
      
      But we don't create ORGIN xattr for broken hardlinks (index=off).  And with
      metacopy feature it is possible that we will install lowerstack[0] but
      ORIGIN xattr is not there.  It is data dentry of upper metacopy dentry
      which has been found using regular name based lookup or using REDIRECT.  So
      with addition of this new case, just presence of oe->numlower is not
      sufficient to guarantee that ORIGIN xattr is present.
      
      So to differentiate between two cases, look at OVL_CONST_INO flag.  If this
      flag is set and upperdentry is there, that means it can be marked as type
      ORIGIN.  OVL_CONST_INO is not set if lower hardlink is broken or will be
      broken over copy up.
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      60124877
    • Vivek Goyal's avatar
      ovl: Treat metacopy dentries as type OVL_PATH_MERGE · 0b17c28a
      Vivek Goyal authored
      
      Right now OVL_PATH_MERGE is used only for merged directories.  But
      conceptually, a metacopy dentry (backed by a lower data dentry) is a merged
      entity as well.
      
      So mark metacopy dentries as OVL_PATH_MERGE and ovl_rename() makes use of
      this property later to set redirect on a metacopy file.
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      0b17c28a
    • Vivek Goyal's avatar
      ovl: Add helper ovl_inode_realdata() · 4823d49c
      Vivek Goyal authored
      
      Add an helper to retrieve real data inode associated with overlay inode.
      This helper will ignore all metacopy inodes and will return only the real
      inode which has data.
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      4823d49c
    • Vivek Goyal's avatar
      ovl: Store lower data inode in ovl_inode · 2664bd08
      Vivek Goyal authored
      
      Right now ovl_inode stores inode pointer for lower inode.  This helps with
      quickly getting lower inode given overlay inode (ovl_inode_lower()).
      
      Now with metadata only copy-up, we can have metacopy inode in middle layer
      as well and inode containing data can be different from ->lower.  I need to
      be able to open the real file in ovl_open_realfile() and for that I need to
      quickly find the lower data inode.
      
      Hence store lower data inode also in ovl_inode.  Also provide an helper
      ovl_inode_lowerdata() to access this field.
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      2664bd08
    • Vivek Goyal's avatar
      ovl: Fix ovl_getattr() to get number of blocks from lower · 67d756c2
      Vivek Goyal authored
      
      If an inode has been copied up metadata only, then we need to query the
      number of blocks from lower and fill up the stat->st_blocks.
      
      We need to be careful about races where we are doing stat on one cpu and
      data copy up is taking place on other cpu.  We want to return
      stat->st_blocks either from lower or stable upper and not something in
      between.  Hence, ovl_has_upperdata() is called first to figure out whether
      block reporting will take place from lower or upper.
      
      We now support metacopy dentries in middle layer.  That means number of
      blocks reporting needs to come from lowest data dentry and this could be
      different from lower dentry.  Hence we end up making a separate
      vfs_getxattr() call for metacopy dentries to get number of blocks.
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      67d756c2
    • Vivek Goyal's avatar
      ovl: Add helper ovl_dentry_lowerdata() to get lower data dentry · 647d253f
      Vivek Goyal authored
      
      Now we have the notion of data dentry and metacopy dentry.
      ovl_dentry_lower() will return uppermost lower dentry, but it could be
      either data or metacopy dentry.  Now we support metacopy dentries in lower
      layers so it is possible that lowerstack[0] is metacopy dentry while
      lowerstack[1] is actual data dentry.
      
      So add an helper which returns lowest most dentry which is supposed to be
      data dentry.
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      647d253f
    • Vivek Goyal's avatar
      ovl: Copy up meta inode data from lowest data inode · 4f93b426
      Vivek Goyal authored
      
      So far lower could not be a meta inode.  So whenever it was time to copy up
      data of a meta inode, we could copy it up from top most lower dentry.
      
      But now lower itself can be a metacopy inode.  That means data copy up
      needs to take place from a data inode in metacopy inode chain.  Find lower
      data inode in the chain and use that for data copy up.
      
      Introduced a helper called ovl_path_lowerdata() to find the lower data
      inode chain.
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      4f93b426
    • Vivek Goyal's avatar
      ovl: Modify ovl_lookup() and friends to lookup metacopy dentry · 9d3dfea3
      Vivek Goyal authored
      
      This patch modifies ovl_lookup() and friends to lookup metacopy dentries.
      It also allows for presence of metacopy dentries in lower layer.
      
      During lookup, check for presence of OVL_XATTR_METACOPY and if not present,
      set OVL_UPPERDATA bit in flags.
      
      We don't support metacopy feature with nfs_export.  So in nfs_export code,
      we set OVL_UPPERDATA flag set unconditionally if upper inode exists.
      
      Do not follow metacopy origin if we find a metacopy only inode and metacopy
      feature is not enabled for that mount.  Like redirect, this can have
      security implications where an attacker could hand craft upper and try to
      gain access to file on lower which it should not have to begin with.
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      9d3dfea3
    • Vivek Goyal's avatar
      ovl: A new xattr OVL_XATTR_METACOPY for file on upper · 0c288874
      Vivek Goyal authored
      
      Now we will have the capability to have upper inodes which might be only
      metadata copy up and data is still on lower inode.  So add a new xattr
      OVL_XATTR_METACOPY to distinguish between two cases.
      
      Presence of OVL_XATTR_METACOPY reflects that file has been copied up
      metadata only and and data will be copied up later from lower origin.  So
      this xattr is set when a metadata copy takes place and cleared when data
      copy takes place.
      
      We also use a bit in ovl_inode->flags to cache OVL_UPPERDATA which reflects
      whether ovl inode has data or not (as opposed to metadata only copy up).
      
      If a file is copied up metadata only and later when same file is opened for
      WRITE, then data copy up takes place.  We copy up data, remove METACOPY
      xattr and then set the UPPERDATA flag in ovl_inode->flags.  While all these
      operations happen with oi->lock held, read side of oi->flags can be
      lockless.  That is another thread on another cpu can check if UPPERDATA
      flag is set or not.
      
      So this gives us an ordering requirement w.r.t UPPERDATA flag.  That is, if
      another cpu sees UPPERDATA flag set, then it should be guaranteed that
      effects of data copy up and remove xattr operations are also visible.
      
      For example.
      
      	CPU1				CPU2
      ovl_open()				acquire(oi->lock)
       ovl_open_maybe_copy_up()                ovl_copy_up_data()
        open_open_need_copy_up()		 vfs_removexattr()
         ovl_already_copied_up()
          ovl_dentry_needs_data_copy_up()	 ovl_set_flag(OVL_UPPERDATA)
           ovl_test_flag(OVL_UPPERDATA)       release(oi->lock)
      
      Say CPU2 is copying up data and in the end sets UPPERDATA flag.  But if
      CPU1 perceives the effects of setting UPPERDATA flag but not the effects of
      preceding operations (ex. upper that is not fully copied up), it will be a
      problem.
      
      Hence this patch introduces smp_wmb() on setting UPPERDATA flag operation
      and smp_rmb() on UPPERDATA flag test operation.
      
      May be some other lock or barrier is already covering it. But I am not sure
      what that is and is it obvious enough that we will not break it in future.
      
      So hence trying to be safe here and introducing barriers explicitly for
      UPPERDATA flag/bit.
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      0c288874
    • Vivek Goyal's avatar
      ovl: Add helper ovl_already_copied_up() · 2002df85
      Vivek Goyal authored
      
      There are couple of places where we need to know if file is already copied
      up (in lockless manner).  Right now its open coded and there are only two
      conditions to check.  Soon this patch series will introduce another
      condition to check and Amir wants to introduce one more.  So introduce a
      helper instead to check this so that code is easier to read.
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      2002df85
  15. Jul 18, 2018
  16. Apr 12, 2018
    • Amir Goldstein's avatar
      ovl: constant st_ino for non-samefs with xino · e487d889
      Amir Goldstein authored
      
      On 64bit systems, when overlay layers are not all on the same fs, but
      all inode numbers of underlying fs are not using the high bits, use the
      high bits to partition the overlay st_ino address space.  The high bits
      hold the fsid (upper fsid is 0).  This way overlay inode numbers are unique
      and all inodes use overlay st_dev.  Inode numbers are also persistent
      for a given layer configuration.
      
      Currently, our only indication for available high ino bits is from a
      filesystem that supports file handles and uses the default encode_fh()
      operation, which encodes a 32bit inode number.
      
      Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      e487d889
    • Amir Goldstein's avatar
      ovl: allocate anon bdev per unique lower fs · 5148626b
      Amir Goldstein authored
      
      Instead of allocating an anonymous bdev per lower layer, allocate
      one anonymous bdev per every unique lower fs that is different than
      upper fs.
      
      Every unique lower fs is assigned an fsid > 0 and the number of
      unique lower fs are stored in ofs->numlowerfs.
      
      The assigned fsid is stored in the lower layer struct and will be
      used also for inode number multiplexing.
      
      Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      5148626b
    • Amir Goldstein's avatar
      ovl: factor out ovl_map_dev_ino() helper · da309e8c
      Amir Goldstein authored
      
      A helper for ovl_getattr() to map the values of st_dev and st_ino
      according to constant st_ino rules.
      
      Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      da309e8c
    • Amir Goldstein's avatar
      ovl: set i_ino to the value of st_ino for NFS export · 695b46e7
      Amir Goldstein authored
      
      Eddie Horng reported that readdir of an overlayfs directory that
      was exported via NFSv3 returns entries with d_type set to DT_UNKNOWN.
      The reason is that while preparing the response for readdirplus, nfsd
      checks inside encode_entryplus_baggage() that a child dentry's inode
      number matches the value of d_ino returns by overlayfs readdir iterator.
      
      Because the overlayfs inodes use arbitrary inode numbers that are not
      correlated with the values of st_ino/d_ino, NFSv3 falls back to not
      encoding d_type. Although this is an allowed behavior, we can fix it for
      the case of all overlayfs layers on the same underlying filesystem.
      
      When NFS export is enabled and d_ino is consistent with st_ino
      (samefs), set the same value also to i_ino in ovl_fill_inode() for all
      overlayfs inodes, nfsd readdirplus sanity checks will pass.
      ovl_fill_inode() may be called from ovl_new_inode(), before real inode
      was created with ino arg 0. In that case, i_ino will be updated to real
      upper inode i_ino on ovl_inode_init() or ovl_inode_update().
      
      Reported-by: default avatarEddie Horng <eddiehorng.tw@gmail.com>
      Tested-by: default avatarEddie Horng <eddiehorng.tw@gmail.com>
      Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
      Fixes: 8383f174 ("ovl: wire up NFS export operations")
      Cc: <stable@vger.kernel.org> #v4.16
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
      695b46e7
Loading