Skip to content

Commit

Permalink
ovl: detect overlapping layers
Browse files Browse the repository at this point in the history
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: syzbot+9c69c282adc4edd2b540@syzkaller.appspotmail.com
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
  • Loading branch information
Amir Goldstein authored and Miklos Szeredi committed May 29, 2019
1 parent b21d9c4 commit 146d62e
Show file tree
Hide file tree
Showing 6 changed files with 229 additions and 17 deletions.
48 changes: 48 additions & 0 deletions fs/overlayfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,54 @@ struct inode *ovl_lookup_inode(struct super_block *sb, struct dentry *real,
return inode;
}

bool ovl_lookup_trap_inode(struct super_block *sb, struct dentry *dir)
{
struct inode *key = d_inode(dir);
struct inode *trap;
bool res;

trap = ilookup5(sb, (unsigned long) key, ovl_inode_test, key);
if (!trap)
return false;

res = IS_DEADDIR(trap) && !ovl_inode_upper(trap) &&
!ovl_inode_lower(trap);

iput(trap);
return res;
}

/*
* Create an inode cache entry for layer root dir, that will intentionally
* fail ovl_verify_inode(), so any lookup that will find some layer root
* will fail.
*/
struct inode *ovl_get_trap_inode(struct super_block *sb, struct dentry *dir)
{
struct inode *key = d_inode(dir);
struct inode *trap;

if (!d_is_dir(dir))
return ERR_PTR(-ENOTDIR);

trap = iget5_locked(sb, (unsigned long) key, ovl_inode_test,
ovl_inode_set, key);
if (!trap)
return ERR_PTR(-ENOMEM);

if (!(trap->i_state & I_NEW)) {
/* Conflicting layer roots? */
iput(trap);
return ERR_PTR(-ELOOP);
}

trap->i_mode = S_IFDIR;
trap->i_flags = S_DEAD;
unlock_new_inode(trap);

return trap;
}

/*
* Does overlay inode need to be hashed by lower inode?
*/
Expand Down
8 changes: 8 additions & 0 deletions fs/overlayfs/namei.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "overlayfs.h"

struct ovl_lookup_data {
struct super_block *sb;
struct qstr name;
bool is_dir;
bool opaque;
Expand Down Expand Up @@ -244,6 +245,12 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
if (!d->metacopy || d->last)
goto out;
} else {
if (ovl_lookup_trap_inode(d->sb, this)) {
/* Caught in a trap of overlapping layers */
err = -ELOOP;
goto out_err;
}

if (last_element)
d->is_dir = true;
if (d->last)
Expand Down Expand Up @@ -819,6 +826,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
int err;
bool metacopy = false;
struct ovl_lookup_data d = {
.sb = dentry->d_sb,
.name = dentry->d_name,
.is_dir = false,
.opaque = false,
Expand Down
3 changes: 3 additions & 0 deletions fs/overlayfs/overlayfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ void ovl_clear_flag(unsigned long flag, struct inode *inode);
bool ovl_test_flag(unsigned long flag, struct inode *inode);
bool ovl_inuse_trylock(struct dentry *dentry);
void ovl_inuse_unlock(struct dentry *dentry);
bool ovl_is_inuse(struct dentry *dentry);
bool ovl_need_index(struct dentry *dentry);
int ovl_nlink_start(struct dentry *dentry);
void ovl_nlink_end(struct dentry *dentry);
Expand Down Expand Up @@ -376,6 +377,8 @@ struct ovl_inode_params {
struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, dev_t rdev);
struct inode *ovl_lookup_inode(struct super_block *sb, struct dentry *real,
bool is_upper);
bool ovl_lookup_trap_inode(struct super_block *sb, struct dentry *dir);
struct inode *ovl_get_trap_inode(struct super_block *sb, struct dentry *dir);
struct inode *ovl_get_inode(struct super_block *sb,
struct ovl_inode_params *oip);
static inline void ovl_copyattr(struct inode *from, struct inode *to)
Expand Down
6 changes: 6 additions & 0 deletions fs/overlayfs/ovl_entry.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ struct ovl_sb {

struct ovl_layer {
struct vfsmount *mnt;
/* Trap in ovl inode cache */
struct inode *trap;
struct ovl_sb *fs;
/* Index of this layer in fs root (upper idx == 0) */
int idx;
Expand Down Expand Up @@ -65,6 +67,10 @@ struct ovl_fs {
/* Did we take the inuse lock? */
bool upperdir_locked;
bool workdir_locked;
/* Traps in ovl inode cache */
struct inode *upperdir_trap;
struct inode *workdir_trap;
struct inode *indexdir_trap;
/* Inode numbers in all layers do not use the high xino_bits */
unsigned int xino_bits;
};
Expand Down
Loading

0 comments on commit 146d62e

Please sign in to comment.