xfs: validate recovered name buffers when recovering xattr items

Strengthen the xattri log item recovery code by checking that we
actually have the required name and newname buffers for whatever
operation we're replaying.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>

diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index b4c2dcb4581bc3df218f8da1f0654b8d02b23f1e..ebd6e98d9c66114b5bc1519b4e243e5727ceb0da 100644 (file)
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -741,22 +741,20 @@ xlog_recover_attri_commit_pass2(
        const void                      *attr_value = NULL;
        const void                      *attr_name;
        size_t                          len;
-       unsigned int                    op;
-
-       attri_formatp = item->ri_buf[0].i_addr;
-       attr_name = item->ri_buf[1].i_addr;
+       unsigned int                    op, i = 0;
 
        /* Validate xfs_attri_log_format before the large memory allocation */
        len = sizeof(struct xfs_attri_log_format);
-       if (item->ri_buf[0].i_len != len) {
+       if (item->ri_buf[i].i_len != len) {
                XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
                                item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
                return -EFSCORRUPTED;
        }
 
+       attri_formatp = item->ri_buf[i].i_addr;
        if (!xfs_attri_validate(mp, attri_formatp)) {
                XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
-                               item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
+                               attri_formatp, len);
                return -EFSCORRUPTED;
        }
 
@@ -785,31 +783,69 @@ xlog_recover_attri_commit_pass2(
                                     attri_formatp, len);
                return -EFSCORRUPTED;
        }
+       i++;
 
        /* Validate the attr name */
-       if (item->ri_buf[1].i_len !=
+       if (item->ri_buf[i].i_len !=
                        xlog_calc_iovec_len(attri_formatp->alfi_name_len)) {
                XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
-                               item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
+                               attri_formatp, len);
                return -EFSCORRUPTED;
        }
 
+       attr_name = item->ri_buf[i].i_addr;
        if (!xfs_attr_namecheck(attr_name, attri_formatp->alfi_name_len)) {
                XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
-                               item->ri_buf[1].i_addr, item->ri_buf[1].i_len);
+                               attri_formatp, len);
                return -EFSCORRUPTED;
        }
+       i++;
 
        /* Validate the attr value, if present */
        if (attri_formatp->alfi_value_len != 0) {
-               if (item->ri_buf[2].i_len != xlog_calc_iovec_len(attri_formatp->alfi_value_len)) {
+               if (item->ri_buf[i].i_len != xlog_calc_iovec_len(attri_formatp->alfi_value_len)) {
                        XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
                                        item->ri_buf[0].i_addr,
                                        item->ri_buf[0].i_len);
                        return -EFSCORRUPTED;
                }
 
-               attr_value = item->ri_buf[2].i_addr;
+               attr_value = item->ri_buf[i].i_addr;
+               i++;
+       }
+
+       /*
+        * Make sure we got the correct number of buffers for the operation
+        * that we just loaded.
+        */
+       if (i != item->ri_total) {
+               XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+                               attri_formatp, len);
+               return -EFSCORRUPTED;
+       }
+
+       switch (op) {
+       case XFS_ATTRI_OP_FLAGS_REMOVE:
+               /* Regular remove operations operate only on names. */
+               if (attr_value != NULL || attri_formatp->alfi_value_len != 0) {
+                       XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+                                            attri_formatp, len);
+                       return -EFSCORRUPTED;
+               }
+               fallthrough;
+       case XFS_ATTRI_OP_FLAGS_SET:
+       case XFS_ATTRI_OP_FLAGS_REPLACE:
+               /*
+                * Regular xattr set/remove/replace operations require a name
+                * and do not take a newname.  Values are optional for set and
+                * replace.
+                */
+               if (attr_name == NULL || attri_formatp->alfi_name_len == 0) {
+                       XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+                                            attri_formatp, len);
+                       return -EFSCORRUPTED;
+               }
+               break;
        }
 
        /*