]> git.ipfire.org Git - thirdparty/kernel/stable.git/commitdiff
xfs: validate recovered name buffers when recovering xattr items
authorDarrick J. Wong <djwong@kernel.org>
Wed, 30 Apr 2025 21:26:53 +0000 (14:26 -0700)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 9 May 2025 07:41:37 +0000 (09:41 +0200)
[ Upstream commit 1c7f09d210aba2f2bb206e2e8c97c9f11a3fd880 ]

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>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
Acked-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
fs/xfs/xfs_attr_item.c

index a8e09ea2622d84ca459fd607f58324cbcc5cb4ed..4a712f1565c1e81eaa1657c27509997346027d91 100644 (file)
@@ -717,22 +717,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;
        }
 
@@ -761,31 +759,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;
        }
 
        /*