Re: [PATCH] erofs-utils: lib: fix erofs_is_packed_inode() on the read path

Gao Xiang <[email protected]>
Newsgroups org.ozlabs.lists.linux-erofs
Message-ID <[email protected]>
Hi Seula,

On Thu, Aug 13, 2026 at 06:38:49PM +0900, [email protected] wrote:
> From: Seula Lee <[email protected]>
> 
> erofs_is_packed_inode() compares i_srcpath against the global
> EROFS_PACKED_INODE identifier.  i_srcpath is only assigned by
> mkfs.erofs and has no on-disk counterpart, so it is never set for
> inodes filled in by erofs_read_inode_from_disk(), and the helper always
> returns false on the read path.
> 
> fsck.erofs therefore treats the packed inode as an ordinary regular
> file at all four of its call sites, for any image with fragments and a
> non-zero packed_nid:
> 
> - erofsfsck_extract_inode(): the guard never fires, so
> erofs_extract_file() writes the packed data to fsckcfg.extract_path
> itself, which is still the top-level directory because main()
> checks the packed inode before the root inode. Extracting the root
> then fails in erofs_extract_dir():
> 
> $ mkfs.erofs -zlz4hc -Efragments img.erofs dir
> $ fsck.erofs --extract=out img.erofs
> <E> erofs: erofs_extract_dir() Line[697] path is not a directory: out
> <E> erofs: main() Line[1256] Failed to extract filesystem
> $ file out
> out: ASCII text # 603720 bytes, the packed inode itself
> 
> - erofsfsck_set_attributes(): the packed inode's mode, timestamps and
> xattrs are applied to the extraction root directory.
> 
> - erofs_verify_inode_data(): the packed inode's own compressed data is
> decoded and hashed as if it were file data.
> 
> - the -p accounting: fragment bytes are counted both as file data and
> as packed inode data, so the reported ratio is too low.
> 
> Restore the nid-based test, which is how the packed inode is already
> identified elsewhere on the read path: z_erofs_read_one_data() compares
> inode->nid against sbi->packed_nid directly, and the
> erofs_sb_has_fragments(sbi) && sbi->packed_nid > 0 idiom used here is
> the same one in fuse/main.c, dump/main.c and fsck.erofs' own
> erofs_packedfile_init() call.
> 
> The i_srcpath comparison has to stay first rather than be replaced:
> while mkfs.erofs builds the packed inode its nid is still
> EROFS_NID_UNALLOCATED and sbi->packed_nid is 0, so a nid-only test
> would return false there and break fragment dedupe and
> pclusterblks_packed handling in lib/compress.c.
> 
> This restores the v1.8.x behaviour at every call site, where the helper
> was nid-based; in particular the packed inode is again excluded from the
> decompression check that --extract performs, as it was before v1.9.
> 
> dump.erofs is affected the same way and prints "Path : (packed file)"
> again as intended.
> 
> erofs_is_metabox_inode() shares the same idiom, but it has no read-path
> caller (all callers are in lib/compress.c and lib/inode.c), so it is
> left alone.
> 
> Fixes: 7928074b7643 ("erofs-utils: introduce metadata compression [metabox]")
> Signed-off-by: Seula Lee <[email protected]>

Thanks for the patch, that is indeed a bug and the fix looks fine,
but the commit message is too long to understand the impacts (it seems
in a LLM-generated-style.)

I will rephrase a short commit message instead.

Thanks,
Gao Xiang
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.