From: Allison Henderson <allison.hender...@oracle.com>

Attribute names of parent pointers are not strings.  So we need to modify
attr_namecheck to verify parent pointer records when the XFS_ATTR_PARENT flag is
set.

Signed-off-by: Allison Henderson <allison.hender...@oracle.com>
Reviewed-by: Darrick J. Wong <djw...@kernel.org>
---
 fs/xfs/libxfs/xfs_attr.c      | 47 ++++++++++++++++++++++++++++++++---
 fs/xfs/libxfs/xfs_attr.h      |  3 ++-
 fs/xfs/libxfs/xfs_da_format.h |  8 ++++++
 fs/xfs/scrub/attr.c           |  2 +-
 fs/xfs/xfs_attr_item.c        | 11 +++++---
 fs/xfs/xfs_attr_list.c        | 17 +++++++++----
 6 files changed, 74 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 101823772bf9..711022742e34 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1577,9 +1577,33 @@ xfs_attr_node_get(
        return error;
 }
 
-/* Returns true if the attribute entry name is valid. */
-bool
-xfs_attr_namecheck(
+/*
+ * Verify parent pointer attribute is valid.
+ * Return true on success or false on failure
+ */
+STATIC bool
+xfs_verify_pptr(
+       struct xfs_mount                        *mp,
+       const struct xfs_parent_name_rec        *rec)
+{
+       xfs_ino_t                               p_ino;
+       xfs_dir2_dataptr_t                      p_diroffset;
+
+       p_ino = be64_to_cpu(rec->p_ino);
+       p_diroffset = be32_to_cpu(rec->p_diroffset);
+
+       if (!xfs_verify_ino(mp, p_ino))
+               return false;
+
+       if (p_diroffset > XFS_DIR2_MAX_DATAPTR)
+               return false;
+
+       return true;
+}
+
+/* Returns true if the string attribute entry name is valid. */
+static bool
+xfs_str_attr_namecheck(
        const void      *name,
        size_t          length)
 {
@@ -1594,6 +1618,23 @@ xfs_attr_namecheck(
        return !memchr(name, 0, length);
 }
 
+/* Returns true if the attribute entry name is valid. */
+bool
+xfs_attr_namecheck(
+       struct xfs_mount        *mp,
+       const void              *name,
+       size_t                  length,
+       int                     flags)
+{
+       if (flags & XFS_ATTR_PARENT) {
+               if (length != sizeof(struct xfs_parent_name_rec))
+                       return false;
+               return xfs_verify_pptr(mp, (struct xfs_parent_name_rec *)name);
+       }
+
+       return xfs_str_attr_namecheck(name, length);
+}
+
 int __init
 xfs_attr_intent_init_cache(void)
 {
diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
index 3e81f3f48560..b79dae788cfb 100644
--- a/fs/xfs/libxfs/xfs_attr.h
+++ b/fs/xfs/libxfs/xfs_attr.h
@@ -547,7 +547,8 @@ int xfs_attr_get(struct xfs_da_args *args);
 int xfs_attr_set(struct xfs_da_args *args);
 int xfs_attr_set_iter(struct xfs_attr_intent *attr);
 int xfs_attr_remove_iter(struct xfs_attr_intent *attr);
-bool xfs_attr_namecheck(const void *name, size_t length);
+bool xfs_attr_namecheck(struct xfs_mount *mp, const void *name, size_t length,
+                       int flags);
 int xfs_attr_calc_size(struct xfs_da_args *args, int *local);
 void xfs_init_attr_trans(struct xfs_da_args *args, struct xfs_trans_res *tres,
                         unsigned int *total);
diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
index b02b67f1999e..75b13807145d 100644
--- a/fs/xfs/libxfs/xfs_da_format.h
+++ b/fs/xfs/libxfs/xfs_da_format.h
@@ -731,6 +731,14 @@ xfs_attr3_leaf_name(xfs_attr_leafblock_t *leafp, int idx)
        return &((char *)leafp)[be16_to_cpu(entries[idx].nameidx)];
 }
 
+static inline int
+xfs_attr3_leaf_flags(xfs_attr_leafblock_t *leafp, int idx)
+{
+       struct xfs_attr_leaf_entry *entries = xfs_attr3_leaf_entryp(leafp);
+
+       return entries[idx].flags;
+}
+
 static inline xfs_attr_leaf_name_remote_t *
 xfs_attr3_leaf_name_remote(xfs_attr_leafblock_t *leafp, int idx)
 {
diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
index 9d2e33743ecd..2a79a13cb600 100644
--- a/fs/xfs/scrub/attr.c
+++ b/fs/xfs/scrub/attr.c
@@ -129,7 +129,7 @@ xchk_xattr_listent(
        }
 
        /* Does this name make sense? */
-       if (!xfs_attr_namecheck(name, namelen)) {
+       if (!xfs_attr_namecheck(sx->sc->mp, name, namelen, flags)) {
                xchk_fblock_set_corrupt(sx->sc, XFS_ATTR_FORK, args.blkno);
                return;
        }
diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index 95e9ecbb4a67..da807f286a09 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -593,7 +593,8 @@ xfs_attri_item_recover(
         */
        attrp = &attrip->attri_format;
        if (!xfs_attri_validate(mp, attrp) ||
-           !xfs_attr_namecheck(nv->name.i_addr, nv->name.i_len))
+           !xfs_attr_namecheck(mp, nv->name.i_addr, nv->name.i_len,
+                               attrp->alfi_attr_filter))
                return -EFSCORRUPTED;
 
        error = xlog_recover_iget(mp,  attrp->alfi_ino, &ip);
@@ -804,7 +805,8 @@ xlog_recover_attri_commit_pass2(
        }
 
        attr_name = item->ri_buf[i].i_addr;
-       if (!xfs_attr_namecheck(attr_name, attri_formatp->alfi_name_len)) {
+       if (!xfs_attr_namecheck(mp, attr_name, attri_formatp->alfi_name_len,
+                               attri_formatp->alfi_attr_filter)) {
                XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
                                item->ri_buf[i].i_addr, item->ri_buf[i].i_len);
                return -EFSCORRUPTED;
@@ -822,8 +824,9 @@ xlog_recover_attri_commit_pass2(
                }
 
                attr_nname = item->ri_buf[i].i_addr;
-               if (!xfs_attr_namecheck(attr_nname,
-                               attri_formatp->alfi_nname_len)) {
+               if (!xfs_attr_namecheck(mp, attr_nname,
+                               attri_formatp->alfi_nname_len,
+                               attri_formatp->alfi_attr_filter)) {
                        XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
                                        item->ri_buf[i].i_addr,
                                        item->ri_buf[i].i_len);
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 99bbbe1a0e44..a51f7f13a352 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -58,9 +58,13 @@ xfs_attr_shortform_list(
        struct xfs_attr_sf_sort         *sbuf, *sbp;
        struct xfs_attr_shortform       *sf;
        struct xfs_attr_sf_entry        *sfe;
+       struct xfs_mount                *mp;
        int                             sbsize, nsbuf, count, i;
        int                             error = 0;
 
+       ASSERT(context != NULL);
+       ASSERT(dp != NULL);
+       mp = dp->i_mount;
        sf = (struct xfs_attr_shortform *)dp->i_af.if_u1.if_data;
        ASSERT(sf != NULL);
        if (!sf->hdr.count)
@@ -82,8 +86,9 @@ xfs_attr_shortform_list(
             (dp->i_af.if_bytes + sf->hdr.count * 16) < context->bufsize)) {
                for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) {
                        if (XFS_IS_CORRUPT(context->dp->i_mount,
-                                          !xfs_attr_namecheck(sfe->nameval,
-                                                              sfe->namelen)))
+                                          !xfs_attr_namecheck(mp, sfe->nameval,
+                                                              sfe->namelen,
+                                                              sfe->flags)))
                                return -EFSCORRUPTED;
                        context->put_listent(context,
                                             sfe->flags,
@@ -174,8 +179,9 @@ xfs_attr_shortform_list(
                        cursor->offset = 0;
                }
                if (XFS_IS_CORRUPT(context->dp->i_mount,
-                                  !xfs_attr_namecheck(sbp->name,
-                                                      sbp->namelen))) {
+                                  !xfs_attr_namecheck(mp, sbp->name,
+                                                      sbp->namelen,
+                                                      sbp->flags))) {
                        error = -EFSCORRUPTED;
                        goto out;
                }
@@ -465,7 +471,8 @@ xfs_attr3_leaf_list_int(
                }
 
                if (XFS_IS_CORRUPT(context->dp->i_mount,
-                                  !xfs_attr_namecheck(name, namelen)))
+                                  !xfs_attr_namecheck(mp, name, namelen,
+                                                      entry->flags)))
                        return -EFSCORRUPTED;
                context->put_listent(context, entry->flags,
                                              name, namelen, valuelen);
-- 
2.38.4

Reply via email to