[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 <chao at 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 <lihongbo22 at 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;
>