Skip to content

Commit

Permalink
ovl: fix EIO from lookup of non-indexed upper
Browse files Browse the repository at this point in the history
Commit fbaf94e ("ovl: don't set origin on broken lower hardlink")
attempt to avoid the condition of non-indexed upper inode with lower
hardlink as origin. If this condition is found, lookup returns EIO.

The protection of commit mentioned above does not cover the case of lower
that is not a hardlink when it is copied up (with either index=off/on)
and then lower is hardlinked while overlay is offline.

Changes to lower layer while overlayfs is offline should not result in
unexpected behavior, so a permanent EIO error after creating a link in
lower layer should not be considered as correct behavior.

This fix replaces EIO error with success in cases where upper has origin
but no index is found, or index is found that does not match upper
inode. In those cases, lookup will not fail and the returned overlay inode
will be hashed by upper inode instead of by lower origin inode.

Fixes: 359f392 ("ovl: lookup index entry for copy up origin")
Cc: <[email protected]> # v4.13
Signed-off-by: Amir Goldstein <[email protected]>
Signed-off-by: Miklos Szeredi <[email protected]>
  • Loading branch information
amir73il authored and Miklos Szeredi committed Oct 24, 2017
1 parent 0ce5cdc commit 6eaf011
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 17 deletions.
20 changes: 16 additions & 4 deletions fs/overlayfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -598,18 +598,30 @@ static bool ovl_verify_inode(struct inode *inode, struct dentry *lowerdentry,
return true;
}

struct inode *ovl_get_inode(struct dentry *dentry, struct dentry *upperdentry)
struct inode *ovl_get_inode(struct dentry *dentry, struct dentry *upperdentry,
struct dentry *index)
{
struct dentry *lowerdentry = ovl_dentry_lower(dentry);
struct inode *realinode = upperdentry ? d_inode(upperdentry) : NULL;
struct inode *inode;
/* Already indexed or could be indexed on copy up? */
bool indexed = (index || (ovl_indexdir(dentry->d_sb) && !upperdentry));

if (WARN_ON(upperdentry && indexed && !lowerdentry))
return ERR_PTR(-EIO);

if (!realinode)
realinode = d_inode(lowerdentry);

if (!S_ISDIR(realinode->i_mode) &&
(upperdentry || (lowerdentry && ovl_indexdir(dentry->d_sb)))) {
struct inode *key = d_inode(lowerdentry ?: upperdentry);
/*
* Copy up origin (lower) may exist for non-indexed upper, but we must
* not use lower as hash key in that case.
* Hash inodes that are or could be indexed by origin inode and
* non-indexed upper inodes that could be hard linked by upper inode.
*/
if (!S_ISDIR(realinode->i_mode) && (upperdentry || indexed)) {
struct inode *key = d_inode(indexed ? lowerdentry :
upperdentry);
unsigned int nlink;

inode = iget5_locked(dentry->d_sb, (unsigned long) key,
Expand Down
20 changes: 8 additions & 12 deletions fs/overlayfs/namei.c
Original file line number Diff line number Diff line change
Expand Up @@ -516,18 +516,9 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,

inode = d_inode(index);
if (d_is_negative(index)) {
if (upper && d_inode(origin)->i_nlink > 1) {
pr_warn_ratelimited("overlayfs: hard link with origin but no index (ino=%lu).\n",
d_inode(origin)->i_ino);
goto fail;
}

dput(index);
index = NULL;
goto out_dput;
} else if (upper && d_inode(upper) != inode) {
pr_warn_ratelimited("overlayfs: wrong index found (index=%pd2, ino=%lu, upper ino=%lu).\n",
index, inode->i_ino, d_inode(upper)->i_ino);
goto fail;
goto out_dput;
} else if (ovl_dentry_weird(index) || ovl_is_whiteout(index) ||
((inode->i_mode ^ d_inode(origin)->i_mode) & S_IFMT)) {
/*
Expand All @@ -547,6 +538,11 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
kfree(name.name);
return index;

out_dput:
dput(index);
index = NULL;
goto out;

fail:
dput(index);
index = ERR_PTR(-EIO);
Expand Down Expand Up @@ -710,7 +706,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
upperdentry = dget(index);

if (upperdentry || ctr) {
inode = ovl_get_inode(dentry, upperdentry);
inode = ovl_get_inode(dentry, upperdentry, index);
err = PTR_ERR(inode);
if (IS_ERR(inode))
goto out_free_oe;
Expand Down
3 changes: 2 additions & 1 deletion fs/overlayfs/overlayfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,8 @@ int ovl_update_time(struct inode *inode, struct timespec *ts, int flags);
bool ovl_is_private_xattr(const char *name);

struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, dev_t rdev);
struct inode *ovl_get_inode(struct dentry *dentry, struct dentry *upperdentry);
struct inode *ovl_get_inode(struct dentry *dentry, struct dentry *upperdentry,
struct dentry *index);
static inline void ovl_copyattr(struct inode *from, struct inode *to)
{
to->i_uid = from->i_uid;
Expand Down

0 comments on commit 6eaf011

Please sign in to comment.