ovl: use ovl_numlower() and ovl_lowerstack() accessors

This helps fortify against dereferencing a NULL ovl_entry,
before we move the ovl_entry reference into ovl_inode.

Reviewed-by: Alexander Larsson <alexl@redhat.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>

diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
index 2cfdfcc..ddb5466 100644 (file)
--- a/fs/overlayfs/export.c
+++ b/fs/overlayfs/export.c
@@ -80,7 +80,7 @@ static int ovl_connectable_layer(struct dentry *dentry)
 
        /* We can get overlay root from root of any layer */
        if (dentry == dentry->d_sb->s_root)
-               return oe->numlower;
+               return ovl_numlower(oe);
 
        /*
         * If it's an unindexed merge dir, then it's not connectable with any
@@ -91,7 +91,7 @@ static int ovl_connectable_layer(struct dentry *dentry)
                return 0;
 
        /* We can get upper/overlay path from indexed/lower dentry */
-       return oe->lowerstack[0].layer->idx;
+       return ovl_lowerstack(oe)->layer->idx;
 }
 
 /*
@@ -105,6 +105,7 @@ static int ovl_connectable_layer(struct dentry *dentry)
 static int ovl_connect_layer(struct dentry *dentry)
 {
        struct dentry *next, *parent = NULL;
+       struct ovl_entry *oe = OVL_E(dentry);
        int origin_layer;
        int err = 0;
 
@@ -112,7 +113,7 @@ static int ovl_connect_layer(struct dentry *dentry)
            WARN_ON(!ovl_dentry_lower(dentry)))
                return -EIO;
 
-       origin_layer = OVL_E(dentry)->lowerstack[0].layer->idx;
+       origin_layer = ovl_lowerstack(oe)->layer->idx;
        if (ovl_dentry_test_flag(OVL_E_CONNECTED, dentry))
                return origin_layer;
 
@@ -319,8 +320,8 @@ static struct dentry *ovl_obtain_alias(struct super_block *sb,
                goto nomem;
 
        if (lower) {
-               oe->lowerstack->dentry = dget(lower);
-               oe->lowerstack->layer = lowerpath->layer;
+               ovl_lowerstack(oe)->dentry = dget(lower);
+               ovl_lowerstack(oe)->layer = lowerpath->layer;
        }
        dentry->d_fsdata = oe;
        if (upper_alias)
@@ -342,14 +343,15 @@ out_iput:
 static struct dentry *ovl_dentry_real_at(struct dentry *dentry, int idx)
 {
        struct ovl_entry *oe = OVL_E(dentry);
+       struct ovl_path *lowerstack = ovl_lowerstack(oe);
        int i;
 
        if (!idx)
                return ovl_dentry_upper(dentry);
 
-       for (i = 0; i < oe->numlower; i++) {
-               if (oe->lowerstack[i].layer->idx == idx)
-                       return oe->lowerstack[i].dentry;
+       for (i = 0; i < ovl_numlower(oe); i++) {
+               if (lowerstack[i].layer->idx == idx)
+                       return lowerstack[i].dentry;
        }
 
        return NULL;

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index e66352f..31f889d 100644 (file)
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -791,19 +791,20 @@ fail:
 int ovl_path_next(int idx, struct dentry *dentry, struct path *path)
 {
        struct ovl_entry *oe = OVL_E(dentry);
+       struct ovl_path *lowerstack = ovl_lowerstack(oe);
 
        BUG_ON(idx < 0);
        if (idx == 0) {
                ovl_path_upper(dentry, path);
                if (path->dentry)
-                       return oe->numlower ? 1 : -1;
+                       return ovl_numlower(oe) ? 1 : -1;
                idx++;
        }
-       BUG_ON(idx > oe->numlower);
-       path->dentry = oe->lowerstack[idx - 1].dentry;
-       path->mnt = oe->lowerstack[idx - 1].layer->mnt;
+       BUG_ON(idx > ovl_numlower(oe));
+       path->dentry = lowerstack[idx - 1].dentry;
+       path->mnt = lowerstack[idx - 1].layer->mnt;
 
-       return (idx < oe->numlower) ? idx + 1 : -1;
+       return (idx < ovl_numlower(oe)) ? idx + 1 : -1;
 }
 
 /* Fix missing 'origin' xattr */
@@ -853,7 +854,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
                .is_dir = false,
                .opaque = false,
                .stop = false,
-               .last = ofs->config.redirect_follow ? false : !poe->numlower,
+               .last = ofs->config.redirect_follow ? false : !ovl_numlower(poe),
                .redirect = NULL,
                .metacopy = false,
        };
@@ -904,7 +905,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
                upperopaque = d.opaque;
        }
 
-       if (!d.stop && poe->numlower) {
+       if (!d.stop && ovl_numlower(poe)) {
                err = -ENOMEM;
                stack = kcalloc(ofs->numlayer - 1, sizeof(struct ovl_path),
                                GFP_KERNEL);
@@ -912,13 +913,13 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
                        goto out_put_upper;
        }
 
-       for (i = 0; !d.stop && i < poe->numlower; i++) {
-               struct ovl_path lower = poe->lowerstack[i];
+       for (i = 0; !d.stop && i < ovl_numlower(poe); i++) {
+               struct ovl_path lower = ovl_lowerstack(poe)[i];
 
                if (!ofs->config.redirect_follow)
-                       d.last = i == poe->numlower - 1;
+                       d.last = i == ovl_numlower(poe) - 1;
                else
-                       d.last = lower.layer->idx == roe->numlower;
+                       d.last = lower.layer->idx == ovl_numlower(roe);
 
                d.mnt = lower.layer->mnt;
                err = ovl_lookup_layer(lower.dentry, &d, &this, false);
@@ -1072,7 +1073,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
        if (!oe)
                goto out_put;
 
-       memcpy(oe->lowerstack, stack, sizeof(struct ovl_path) * ctr);
+       memcpy(ovl_lowerstack(oe), stack, sizeof(struct ovl_path) * ctr);
        dentry->d_fsdata = oe;
 
        if (upperopaque)
@@ -1177,12 +1178,13 @@ bool ovl_lower_positive(struct dentry *dentry)
 
        old_cred = ovl_override_creds(dentry->d_sb);
        /* Positive upper -> have to look up lower to see whether it exists */
-       for (i = 0; !done && !positive && i < poe->numlower; i++) {
+       for (i = 0; !done && !positive && i < ovl_numlower(poe); i++) {
                struct dentry *this;
-               struct dentry *lowerdir = poe->lowerstack[i].dentry;
+               struct ovl_path *parentpath = &ovl_lowerstack(poe)[i];
 
-               this = lookup_one_positive_unlocked(mnt_idmap(poe->lowerstack[i].layer->mnt),
-                                                  name->name, lowerdir, name->len);
+               this = lookup_one_positive_unlocked(
+                               mnt_idmap(parentpath->layer->mnt),
+                               name->name, parentpath->dentry, name->len);
                if (IS_ERR(this)) {
                        switch (PTR_ERR(this)) {
                        case -ENOENT:

diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 4c73121..0e009fa 100644 (file)
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -113,10 +113,20 @@ struct ovl_entry {
                };
                struct rcu_head rcu;
        };
-       unsigned numlower;
-       struct ovl_path lowerstack[];
+       unsigned int __numlower;
+       struct ovl_path __lowerstack[];
 };
 
+static inline unsigned int ovl_numlower(struct ovl_entry *oe)
+{
+       return oe ? oe->__numlower : 0;
+}
+
+static inline struct ovl_path *ovl_lowerstack(struct ovl_entry *oe)
+{
+       return ovl_numlower(oe) ? oe->__lowerstack : NULL;
+}
+
 struct ovl_entry *ovl_alloc_entry(unsigned int numlower);
 
 static inline struct ovl_entry *OVL_E(struct dentry *dentry)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index ea34edd..baa970a 100644 (file)
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -56,10 +56,11 @@ MODULE_PARM_DESC(xino_auto,
 
 static void ovl_entry_stack_free(struct ovl_entry *oe)
 {
+       struct ovl_path *lowerstack = ovl_lowerstack(oe);
        unsigned int i;
 
-       for (i = 0; i < oe->numlower; i++)
-               dput(oe->lowerstack[i].dentry);
+       for (i = 0; i < ovl_numlower(oe); i++)
+               dput(lowerstack[i].dentry);
 }
 
 static bool ovl_metacopy_def = IS_ENABLED(CONFIG_OVERLAY_FS_METACOPY);
@@ -139,6 +140,7 @@ static int ovl_dentry_revalidate_common(struct dentry *dentry,
                                        unsigned int flags, bool weak)
 {
        struct ovl_entry *oe = OVL_E(dentry);
+       struct ovl_path *lowerstack = ovl_lowerstack(oe);
        struct inode *inode = d_inode_rcu(dentry);
        struct dentry *upper;
        unsigned int i;
@@ -152,10 +154,9 @@ static int ovl_dentry_revalidate_common(struct dentry *dentry,
        if (upper)
                ret = ovl_revalidate_real(upper, flags, weak);
 
-       for (i = 0; ret > 0 && i < oe->numlower; i++) {
-               ret = ovl_revalidate_real(oe->lowerstack[i].dentry, flags,
-                                         weak);
-       }
+       for (i = 0; ret > 0 && i < ovl_numlower(oe); i++)
+               ret = ovl_revalidate_real(lowerstack[i].dentry, flags, weak);
+
        return ret;
 }
 
@@ -1454,7 +1455,7 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
 
        /* Verify lower root is upper root origin */
        err = ovl_verify_origin(ofs, upperpath->dentry,
-                               oe->lowerstack[0].dentry, true);
+                               ovl_lowerstack(oe)->dentry, true);
        if (err) {
                pr_err("failed to verify upper root origin\n");
                goto out;
@@ -1717,6 +1718,7 @@ static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb,
 {
        int err;
        struct path *stack = NULL;
+       struct ovl_path *lowerstack;
        unsigned int i;
        struct ovl_entry *oe;
 
@@ -1754,9 +1756,10 @@ static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb,
        if (!oe)
                goto out_err;
 
+       lowerstack = ovl_lowerstack(oe);
        for (i = 0; i < numlower; i++) {
-               oe->lowerstack[i].dentry = dget(stack[i].dentry);
-               oe->lowerstack[i].layer = &ofs->layers[i+1];
+               lowerstack[i].dentry = dget(stack[i].dentry);
+               lowerstack[i].layer = &ofs->layers[i+1];
        }
 
 out:
@@ -1849,7 +1852,7 @@ static struct dentry *ovl_get_root(struct super_block *sb,
                                   struct ovl_entry *oe)
 {
        struct dentry *root;
-       struct ovl_path *lowerpath = &oe->lowerstack[0];
+       struct ovl_path *lowerpath = ovl_lowerstack(oe);
        unsigned long ino = d_inode(lowerpath->dentry)->i_ino;
        int fsid = lowerpath->layer->fsid;
        struct ovl_inode_params oip = {

diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 73a5316..a146c7a 100644 (file)
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -85,11 +85,11 @@ bool ovl_verify_lower(struct super_block *sb)
 
 struct ovl_entry *ovl_alloc_entry(unsigned int numlower)
 {
-       size_t size = offsetof(struct ovl_entry, lowerstack[numlower]);
+       size_t size = offsetof(struct ovl_entry, __lowerstack[numlower]);
        struct ovl_entry *oe = kzalloc(size, GFP_KERNEL);
 
        if (oe)
-               oe->numlower = numlower;
+               oe->__numlower = numlower;
 
        return oe;
 }
@@ -120,12 +120,13 @@ void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry,
                           unsigned int mask)
 {
        struct ovl_entry *oe = OVL_E(dentry);
+       struct ovl_path *lowerstack = ovl_lowerstack(oe);
        unsigned int i, flags = 0;
 
        if (upperdentry)
                flags |= upperdentry->d_flags;
-       for (i = 0; i < oe->numlower; i++)
-               flags |= oe->lowerstack[i].dentry->d_flags;
+       for (i = 0; i < ovl_numlower(oe); i++)
+               flags |= lowerstack[i].dentry->d_flags;
 
        spin_lock(&dentry->d_lock);
        dentry->d_flags &= ~mask;
@@ -152,7 +153,7 @@ enum ovl_path_type ovl_path_type(struct dentry *dentry)
                /*
                 * Non-dir dentry can hold lower dentry of its copy up origin.
                 */
-               if (oe->numlower) {
+               if (ovl_numlower(oe)) {
                        if (ovl_test_flag(OVL_CONST_INO, d_inode(dentry)))
                                type |= __OVL_PATH_ORIGIN;
                        if (d_is_dir(dentry) ||
@@ -160,7 +161,7 @@ enum ovl_path_type ovl_path_type(struct dentry *dentry)
                                type |= __OVL_PATH_MERGE;
                }
        } else {
-               if (oe->numlower > 1)
+               if (ovl_numlower(oe) > 1)
                        type |= __OVL_PATH_MERGE;
        }
        return type;
@@ -177,10 +178,11 @@ void ovl_path_upper(struct dentry *dentry, struct path *path)
 void ovl_path_lower(struct dentry *dentry, struct path *path)
 {
        struct ovl_entry *oe = OVL_E(dentry);
+       struct ovl_path *lowerpath = ovl_lowerstack(oe);
 
-       if (oe->numlower) {
-               path->mnt = oe->lowerstack[0].layer->mnt;
-               path->dentry = oe->lowerstack[0].dentry;
+       if (ovl_numlower(oe)) {
+               path->mnt = lowerpath->layer->mnt;
+               path->dentry = lowerpath->dentry;
        } else {
                *path = (struct path) { };
        }
@@ -189,10 +191,11 @@ void ovl_path_lower(struct dentry *dentry, struct path *path)
 void ovl_path_lowerdata(struct dentry *dentry, struct path *path)
 {
        struct ovl_entry *oe = OVL_E(dentry);
+       struct ovl_path *lowerstack = ovl_lowerstack(oe);
 
-       if (oe->numlower) {
-               path->mnt = oe->lowerstack[oe->numlower - 1].layer->mnt;
-               path->dentry = oe->lowerstack[oe->numlower - 1].dentry;
+       if (ovl_numlower(oe)) {
+               path->mnt = lowerstack[ovl_numlower(oe) - 1].layer->mnt;
+               path->dentry = lowerstack[ovl_numlower(oe) - 1].dentry;
        } else {
                *path = (struct path) { };
        }
@@ -233,14 +236,14 @@ struct dentry *ovl_dentry_lower(struct dentry *dentry)
 {
        struct ovl_entry *oe = OVL_E(dentry);
 
-       return oe->numlower ? oe->lowerstack[0].dentry : NULL;
+       return ovl_numlower(oe) ? ovl_lowerstack(oe)->dentry : NULL;
 }
 
 const struct ovl_layer *ovl_layer_lower(struct dentry *dentry)
 {
        struct ovl_entry *oe = OVL_E(dentry);
 
-       return oe->numlower ? oe->lowerstack[0].layer : NULL;
+       return ovl_numlower(oe) ? ovl_lowerstack(oe)->layer : NULL;
 }
 
 /*
@@ -253,7 +256,8 @@ struct dentry *ovl_dentry_lowerdata(struct dentry *dentry)
 {
        struct ovl_entry *oe = OVL_E(dentry);
 
-       return oe->numlower ? oe->lowerstack[oe->numlower - 1].dentry : NULL;
+       return ovl_numlower(oe) ?
+               ovl_lowerstack(oe)[ovl_numlower(oe) - 1].dentry : NULL;
 }
 
 struct dentry *ovl_dentry_real(struct dentry *dentry)
@@ -1028,7 +1032,7 @@ bool ovl_is_metacopy_dentry(struct dentry *dentry)
                return false;
        }
 
-       return (oe->numlower > 1);
+       return (ovl_numlower(oe) > 1);
 }
 
 char *ovl_get_redirect_xattr(struct ovl_fs *ofs, const struct path *path, int padding)