Re: [PATCH] erofs: do sanity check on m->type in z_erofs_load_compact_lcluster()

[Hongbo Li]

On 2025/7/8 10:30, Hongbo Li wrote:

On 2025/7/7 16:47, Chao Yu wrote:

All below functions will do sanity check on m->type, let's move sanity
check to z_erofs_load_compact_lcluster() for cleanup.
- z_erofs_map_blocks_fo
- z_erofs_get_extent_compressedlen
- z_erofs_get_extent_decompressedlen
- z_erofs_extent_lookback

Signed-off-by: Chao Yu <c...@kernel.org>
---
  fs/erofs/zmap.c | 60 ++++++++++++++++++-------------------------------
  1 file changed, 22 insertions(+), 38 deletions(-)

diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 0bebc6e3a4d7..e530b152e14e 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c

@@ -240,6 +240,13 @@ static int z_erofs_load_compact_lcluster(struct z_erofs_maprecorder *m,   static int z_erofs_load_lcluster_from_disk(struct z_erofs_maprecorder *m,

                         unsigned int lcn, bool lookahead)
  {
+    if (m->type >= Z_EROFS_LCLUSTER_TYPE_MAX) {

+        erofs_err(m->inode->i_sb, "unknown type %u @ lcn %u of nid %llu",

+                m->type, lcn, EROFS_I(m->inode)->nid);
+        DBG_BUGON(1);
+        return -EOPNOTSUPP;
+    }
+

Hi, Chao,

After moving the condition in here, there is no need to check in z_erofs_extent_lookback, z_erofs_get_extent_compressedlen and z_erofs_get_extent_decompressedlen. Because in z_erofs_map_blocks_fo, the condition has been checked in before. Right?

So,

Reviewed-by: Hongbo Li <lihongb...@huawei.com>

Thanks,
Hongbo

Thanks,
Hongbo

      switch (EROFS_I(m->inode)->datalayout) {
      case EROFS_INODE_COMPRESSED_FULL:
          return z_erofs_load_full_lcluster(m, lcn);

@@ -265,12 +272,7 @@ static int z_erofs_extent_lookback(struct z_erofs_maprecorder *m,

          if (err)
              return err;
-        if (m->type >= Z_EROFS_LCLUSTER_TYPE_MAX) {
-            erofs_err(sb, "unknown type %u @ lcn %lu of nid %llu",
-                  m->type, lcn, vi->nid);
-            DBG_BUGON(1);
-            return -EOPNOTSUPP;
-        } else if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
+        if (m->type == Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
              lookback_distance = m->delta[0];
              if (!lookback_distance)
                  break;

@@ -333,17 +335,13 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,

          }
          if (m->compressedblks)
              goto out;
-    } else if (m->type < Z_EROFS_LCLUSTER_TYPE_MAX) {
-        /*
-         * if the 1st NONHEAD lcluster is actually PLAIN or HEAD type
-         * rather than CBLKCNT, it's a 1 block-sized pcluster.
-         */
-        m->compressedblks = 1;
-        goto out;
      }

-    erofs_err(sb, "cannot found CBLKCNT @ lcn %lu of nid %llu", lcn, vi->nid);

-    DBG_BUGON(1);
-    return -EFSCORRUPTED;
+
+    /*
+     * if the 1st NONHEAD lcluster is actually PLAIN or HEAD type rather
+     * than CBLKCNT, it's a 1 block-sized pcluster.
+     */
+    m->compressedblks = 1;
  out:
      m->map->m_plen = erofs_pos(sb, m->compressedblks);
      return 0;

@@ -379,11 +377,6 @@ static int z_erofs_get_extent_decompressedlen(struct z_erofs_maprecorder *m)

              if (lcn != headlcn)
                  break;    /* ends at the next HEAD lcluster */
              m->delta[1] = 1;
-        } else {

-            erofs_err(inode->i_sb, "unknown type %u @ lcn %llu of nid %llu",

-                  m->type, lcn, vi->nid);
-            DBG_BUGON(1);
-            return -EOPNOTSUPP;
          }
          lcn += m->delta[1];
      }

@@ -429,10 +422,7 @@ static int z_erofs_map_blocks_fo(struct inode *inode,

      map->m_flags = EROFS_MAP_MAPPED | EROFS_MAP_ENCODED;
      end = (m.lcn + 1ULL) << lclusterbits;
-    switch (m.type) {
-    case Z_EROFS_LCLUSTER_TYPE_PLAIN:
-    case Z_EROFS_LCLUSTER_TYPE_HEAD1:
-    case Z_EROFS_LCLUSTER_TYPE_HEAD2:
+    if (m.type != Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
          if (endoff >= m.clusterofs) {
              m.headtype = m.type;
              map->m_la = (m.lcn << lclusterbits) | m.clusterofs;
@@ -443,7 +433,7 @@ static int z_erofs_map_blocks_fo(struct inode *inode,
               */
              if (ztailpacking && end > inode->i_size)
                  end = inode->i_size;
-            break;
+            goto map_block;
          }
          /* m.lcn should be >= 1 if endoff < m.clusterofs */
          if (!m.lcn) {

@@ -455,19 +445,13 @@ static int z_erofs_map_blocks_fo(struct inode *inode,

          end = (m.lcn << lclusterbits) | m.clusterofs;
          map->m_flags |= EROFS_MAP_FULL_MAPPED;
          m.delta[0] = 1;
-        fallthrough;
-    case Z_EROFS_LCLUSTER_TYPE_NONHEAD:
-        /* get the corresponding first chunk */
-        err = z_erofs_extent_lookback(&m, m.delta[0]);
-        if (err)
-            goto unmap_out;
-        break;
-    default:
-        erofs_err(sb, "unknown type %u @ offset %llu of nid %llu",
-              m.type, ofs, vi->nid);
-        err = -EOPNOTSUPP;
-        goto unmap_out;
      }
+
+    /* get the corresponding first chunk */
+    err = z_erofs_extent_lookback(&m, m.delta[0]);
+    if (err)
+        goto unmap_out;
+map_block:
      if (m.partialref)
          map->m_flags |= EROFS_MAP_PARTIAL_REF;
      map->m_llen = end - map->m_la;