Re: [PATCH v10 0/2] Introduce EROFS support

[Daniel Kiper]

Hey,

On Thu, May 02, 2024 at 03:01:37PM +0800, Yifan Zhao wrote:
> EROFS [1] is a lightweight read-only filesystem designed for performance
> which has already been shipped in most Linux distributions as well as widely
> used in several scenarios, such as Android system partitions, container
> images, and rootfs for embedded devices.
>
> This patch brings EROFS uncompressed support together with related tests.
> Now, it's possible to boot directly through GRUB with an EROFS rootfs.
>
> EROFS compressed files will be supported later since it has more work to
> polish.
>
> [1] https://erofs.docs.kernel.org

Sadly Coverity complains about the EROFS code. Please take a look below...

Daniel

________________________________________________________________________________________________________
*** CID 460612:    (BAD_SHIFT)
/grub-core/fs/erofs.c: 418 in erofs_map_blocks_chunkmode()
412       pos &= ~(unit - 1);
413
414       /* no overflow for multiplication as chunkbits >= 9 and sizeof(unit) 
<= 8 */
415       if (grub_add (pos, chunknr * unit, &pos))
416         return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected");
417
>>>     CID 460612:    (BAD_SHIFT)
>>>     In expression "chunknr << chunkbits", left shifting by more than 63 
>>> bits has undefined behavior.  The shift amount, "chunkbits", is as much as 
>>> 64.
418       map->m_la = chunknr << chunkbits;
419
420       if (grub_sub (erofs_inode_file_size (node), map->m_la, &map->m_plen))
421         return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected");
422       map->m_plen = grub_min (((grub_uint64_t) 1) << chunkbits,
423                               ALIGN_UP (map->m_plen, erofs_blocksz 
(node->data)));
/grub-core/fs/erofs.c: 403 in erofs_map_blocks_chunkmode()
397
398       chunkbits = node->data->sb.log2_blksz + (chunk_format & 
EROFS_CHUNK_FORMAT_BLKBITS_MASK);
399       if (chunkbits > 64)
400         return grub_error (GRUB_ERR_BAD_FS, "invalid chunkbits %u @ inode 
%" PRIuGRUB_UINT64_T,
401                            chunkbits, node->ino);
402
>>>     CID 460612:    (BAD_SHIFT)
>>>     In expression "map->m_la >> chunkbits", right shifting by more than 63 
>>> bits has undefined behavior.  The shift amount, "chunkbits", is as much as 
>>> 64.
403       chunknr = map->m_la >> chunkbits;
404
405       if (grub_add (erofs_iloc (node), erofs_inode_size (node), &pos) ||
406           grub_add (pos, erofs_inode_xattr_ibody_size (node), &pos))
407         return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected");
408
/grub-core/fs/erofs.c: 422 in erofs_map_blocks_chunkmode()
416         return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected");
417
418       map->m_la = chunknr << chunkbits;
419
420       if (grub_sub (erofs_inode_file_size (node), map->m_la, &map->m_plen))
421         return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected");
>>>     CID 460612:    (BAD_SHIFT)
>>>     In expression "1UL << chunkbits", left shifting by more than 63 bits 
>>> has undefined behavior.  The shift amount, "chunkbits", is as much as 64.
422       map->m_plen = grub_min (((grub_uint64_t) 1) << chunkbits,
423                               ALIGN_UP (map->m_plen, erofs_blocksz 
(node->data)));
424
425       if (chunk_format & EROFS_CHUNK_FORMAT_INDEXES)
426         {
427           struct grub_erofs_inode_chunk_index idx;

________________________________________________________________________________________________________
*** CID 460611:  Error handling issues  (CHECKED_RETURN)
/grub-core/fs/erofs.c: 793 in erofs_dir_iter()
787     {
788       struct grub_erofs_dir_ctx *ctx = data;
789       struct grub_dirhook_info info = {0};
790
791       if (!node->inode_loaded)
792         {
>>>     CID 460611:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "erofs_read_inode" without checking return value (as is done 
>>> elsewhere 6 out of 7 times).
793           erofs_read_inode (ctx->data, node);
794           grub_errno = GRUB_ERR_NONE;
795         }
796
797       if (node->inode_loaded)
798         {

________________________________________________________________________________________________________
*** CID 460610:  Resource leaks  (RESOURCE_LEAK)
/grub-core/fs/erofs.c: 957 in grub_erofs_label()
951           *label = NULL;
952           return grub_errno;
953         }
954
955       *label = grub_strndup ((char *) data->sb.volume_name, sizeof 
(data->sb.volume_name));
956       if (!*label)
>>>     CID 460610:  Resource leaks  (RESOURCE_LEAK)
>>>     Variable "data" going out of scope leaks the storage it points to.
957         return grub_errno;
958
959       grub_free (data);
960
961       return GRUB_ERR_NONE;
962     }
________________________________________________________________________________________________________