[PATCH 1/2] erofs: clean up encoded map flags

[PATCH 1/2] erofs: clean up encoded map flags

[Gao Xiang]

- Remove EROFS_MAP_ENCODED since it was always set together with
  EROFS_MAP_MAPPED for compressed extents and checked redundantly;

- Replace the EROFS_MAP_FULL_MAPPED flag with the opposite
  EROFS_MAP_PARTIAL_MAPPED flag so that extents are implicitly
  fully mapped initially to simplify the logic;

- Make fragment extents independent of EROFS_MAP_MAPPED since
  they are not directly allocated on disk; thus fragment extents
  are no longer twisted with mapped extents.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/internal.h          | 23 +++++++++++------------
 fs/erofs/zdata.c             | 19 +++++++++----------
 fs/erofs/zmap.c              | 19 ++++++++++---------
 include/trace/events/erofs.h |  7 +++----
 4 files changed, 33 insertions(+), 35 deletions(-)

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index a4f0a42cf8c3..4792490161ec 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -360,20 +360,19 @@ static inline struct folio *erofs_grab_folio_nowait(struct address_space *as,
 			readahead_gfp_mask(as) & ~__GFP_RECLAIM);
 }
 
-/* Has a disk mapping */
-#define EROFS_MAP_MAPPED	0x0001
+/* Allocated on disk at @m_pa (e.g. NOT a fragment extent) */
+#define EROFS_MAP_MAPPED		0x0001
 /* Located in metadata (could be copied from bd_inode) */
-#define EROFS_MAP_META		0x0002
-/* The extent is encoded */
-#define EROFS_MAP_ENCODED	0x0004
-/* The length of extent is full */
-#define EROFS_MAP_FULL_MAPPED	0x0008
+#define EROFS_MAP_META			0x0002
+/* @m_llen may be truncated by the runtime compared to the on-disk record */
+#define EROFS_MAP_PARTIAL_MAPPED	0x0004
+/* The on-disk @m_llen may cover only part of the encoded data */
+#define EROFS_MAP_PARTIAL_REF		0x0008
 /* Located in the special packed inode */
-#define __EROFS_MAP_FRAGMENT	0x0010
-/* The extent refers to partial decompressed data */
-#define EROFS_MAP_PARTIAL_REF	0x0020
-
-#define EROFS_MAP_FRAGMENT	(EROFS_MAP_MAPPED | __EROFS_MAP_FRAGMENT)
+#define EROFS_MAP_FRAGMENT		0x0010
+/* The encoded on-disk data will be fully handled (decompressed) */
+#define EROFS_MAP_FULL(f)	(!((f) & (EROFS_MAP_PARTIAL_MAPPED | \
+					  EROFS_MAP_PARTIAL_REF)))
 
 struct erofs_map_blocks {
 	struct erofs_buf buf;
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index b566996a0d1a..8a0b15511931 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -520,7 +520,7 @@ static bool z_erofs_should_alloc_cache(struct z_erofs_frontend *fe)
 	if (cachestrategy <= EROFS_ZIP_CACHE_DISABLED)
 		return false;
 
-	if (!(fe->map.m_flags & EROFS_MAP_FULL_MAPPED))
+	if (fe->map.m_flags & EROFS_MAP_PARTIAL_MAPPED)
 		return true;
 
 	if (cachestrategy >= EROFS_ZIP_CACHE_READAROUND &&
@@ -1033,10 +1033,7 @@ static int z_erofs_scan_folio(struct z_erofs_frontend *f,
 		/* bump split parts first to avoid several separate cases */
 		++split;
 
-		if (!(map->m_flags & EROFS_MAP_MAPPED)) {
-			folio_zero_segment(folio, cur, end);
-			tight = false;
-		} else if (map->m_flags & __EROFS_MAP_FRAGMENT) {
+		if (map->m_flags & EROFS_MAP_FRAGMENT) {
 			erofs_off_t fpos = offset + cur - map->m_la;
 
 			err = z_erofs_read_fragment(inode->i_sb, folio, cur,
@@ -1045,6 +1042,9 @@ static int z_erofs_scan_folio(struct z_erofs_frontend *f,
 			if (err)
 				break;
 			tight = false;
+		} else if (!(map->m_flags & EROFS_MAP_MAPPED)) {
+			folio_zero_segment(folio, cur, end);
+			tight = false;
 		} else {
 			if (!f->pcl) {
 				err = z_erofs_pcluster_begin(f);
@@ -1080,14 +1080,13 @@ static int z_erofs_scan_folio(struct z_erofs_frontend *f,
 				f->pcl->length = offset + end - map->m_la;
 				f->pcl->pageofs_out = map->m_la & ~PAGE_MASK;
 			}
-			if ((map->m_flags & EROFS_MAP_FULL_MAPPED) &&
-			    !(map->m_flags & EROFS_MAP_PARTIAL_REF) &&
+			if (EROFS_MAP_FULL(map->m_flags) &&
 			    f->pcl->length == map->m_llen)
 				f->pcl->partial = false;
 		}
 		/* shorten the remaining extent to update progress */
 		map->m_llen = offset + cur - map->m_la;
-		map->m_flags &= ~EROFS_MAP_FULL_MAPPED;
+		map->m_flags |= EROFS_MAP_PARTIAL_MAPPED;
 		if (cur <= pgs) {
 			split = cur < pgs;
 			tight = (bs == PAGE_SIZE);
@@ -1841,7 +1840,7 @@ static void z_erofs_pcluster_readmore(struct z_erofs_frontend *f,
 		map->m_la = end;
 		err = z_erofs_map_blocks_iter(inode, map,
 					      EROFS_GET_BLOCKS_READMORE);
-		if (err || !(map->m_flags & EROFS_MAP_ENCODED))
+		if (err || !(map->m_flags & EROFS_MAP_MAPPED))
 			return;
 
 		/* expand ra for the trailing edge if readahead */
@@ -1853,7 +1852,7 @@ static void z_erofs_pcluster_readmore(struct z_erofs_frontend *f,
 		end = round_up(end, PAGE_SIZE);
 	} else {
 		end = round_up(map->m_la, PAGE_SIZE);
-		if (!(map->m_flags & EROFS_MAP_ENCODED) || !map->m_llen)
+		if (!(map->m_flags & EROFS_MAP_MAPPED) || !map->m_llen)
 			return;
 	}
 
diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 30775502b56d..67f55b9b57af 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -419,7 +419,7 @@ static int z_erofs_map_blocks_fo(struct inode *inode,
 
 	if ((flags & EROFS_GET_BLOCKS_FINDTAIL) && ztailpacking)
 		vi->z_fragmentoff = m.nextpackoff;
-	map->m_flags = EROFS_MAP_MAPPED | EROFS_MAP_ENCODED;
+	map->m_flags = EROFS_MAP_MAPPED | EROFS_MAP_PARTIAL_MAPPED;
 	end = (m.lcn + 1ULL) << lclusterbits;
 
 	if (m.type != Z_EROFS_LCLUSTER_TYPE_NONHEAD && endoff >= m.clusterofs) {
@@ -435,7 +435,7 @@ static int z_erofs_map_blocks_fo(struct inode *inode,
 	} else {
 		if (m.type != Z_EROFS_LCLUSTER_TYPE_NONHEAD) {
 			end = (m.lcn << lclusterbits) | m.clusterofs;
-			map->m_flags |= EROFS_MAP_FULL_MAPPED;
+			map->m_flags &= ~EROFS_MAP_PARTIAL_MAPPED;
 			m.delta[0] = 1;
 		}
 		/* get the corresponding first chunk */
@@ -496,7 +496,7 @@ static int z_erofs_map_blocks_fo(struct inode *inode,
 	      map->m_llen >= i_blocksize(inode))) {
 		err = z_erofs_get_extent_decompressedlen(&m);
 		if (!err)
-			map->m_flags |= EROFS_MAP_FULL_MAPPED;
+			map->m_flags &= ~EROFS_MAP_PARTIAL_MAPPED;
 	}
 
 unmap_out:
@@ -594,8 +594,7 @@ static int z_erofs_map_blocks_ext(struct inode *inode,
 			if (recsz > offsetof(struct z_erofs_extent, pstart_lo))
 				vi->z_fragmentoff |= map->m_pa << 32;
 		} else if (map->m_plen & Z_EROFS_EXTENT_PLEN_MASK) {
-			map->m_flags |= EROFS_MAP_MAPPED |
-				EROFS_MAP_FULL_MAPPED | EROFS_MAP_ENCODED;
+			map->m_flags |= EROFS_MAP_MAPPED;
 			fmt = map->m_plen >> Z_EROFS_EXTENT_PLEN_FMT_BIT;
 			if (map->m_plen & Z_EROFS_EXTENT_PLEN_PARTIAL)
 				map->m_flags |= EROFS_MAP_PARTIAL_REF;
@@ -714,7 +713,7 @@ static int z_erofs_map_sanity_check(struct inode *inode,
 	struct erofs_sb_info *sbi = EROFS_I_SB(inode);
 	u64 pend;
 
-	if (!(map->m_flags & EROFS_MAP_ENCODED))
+	if (!(map->m_flags & EROFS_MAP_MAPPED))
 		return 0;
 	if (unlikely(map->m_algorithmformat >= Z_EROFS_COMPRESSION_RUNTIME_MAX)) {
 		erofs_err(inode->i_sb, "unknown algorithm %d @ pos %llu for nid %llu, please upgrade kernel",
@@ -781,10 +780,12 @@ static int z_erofs_iomap_begin_report(struct inode *inode, loff_t offset,
 	iomap->bdev = inode->i_sb->s_bdev;
 	iomap->offset = map.m_la;
 	iomap->length = map.m_llen;
-	if (map.m_flags & EROFS_MAP_MAPPED) {
+	if (map.m_flags & EROFS_MAP_FRAGMENT) {
 		iomap->type = IOMAP_MAPPED;
-		iomap->addr = map.m_flags & __EROFS_MAP_FRAGMENT ?
-			      IOMAP_NULL_ADDR : map.m_pa;
+		iomap->addr = IOMAP_NULL_ADDR;
+	} else if (map.m_flags & EROFS_MAP_MAPPED) {
+		iomap->type = IOMAP_MAPPED;
+		iomap->addr = map.m_pa;
 	} else {
 		iomap->type = IOMAP_HOLE;
 		iomap->addr = IOMAP_NULL_ADDR;
diff --git a/include/trace/events/erofs.h b/include/trace/events/erofs.h
index def20d06507b..cd0e3fd8c23f 100644
--- a/include/trace/events/erofs.h
+++ b/include/trace/events/erofs.h
@@ -26,10 +26,9 @@ struct erofs_map_blocks;
 #define show_mflags(flags) __print_flags(flags, "",	\
 	{ EROFS_MAP_MAPPED,		"M" },		\
 	{ EROFS_MAP_META,		"I" },		\
-	{ EROFS_MAP_ENCODED,		"E" },		\
-	{ EROFS_MAP_FULL_MAPPED,	"F" },		\
-	{ EROFS_MAP_FRAGMENT,		"R" },		\
-	{ EROFS_MAP_PARTIAL_REF,	"P" })
+	{ EROFS_MAP_PARTIAL_MAPPED,	"T" },		\
+	{ EROFS_MAP_PARTIAL_REF,	"P" },		\
+	{ EROFS_MAP_FRAGMENT,		"R" })
 
 TRACE_EVENT(erofs_lookup,
 
-- 
2.43.5

[PATCH 2/2] erofs: error out obviously illegal extents in advance

[Gao Xiang]

Detect some corrupted extent cases during metadata parsing rather
than letting them result in harmless decompression failures later:

 - For full-reference compressed extents, the compressed size must
   not exceed the decompressed size, which is a strict on-disk
   layout constraint;

 - For plain (shifted/interlaced) extents, the decoded size must
   not exceed the encoded size, even accounting for partial decoding.

Both ways work but it should be better to report illegal extents as
metadata layout violations rather than deferring as decompression
failure.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/decompressor.c |  1 -
 fs/erofs/zmap.c         | 24 +++++++++++++++---------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index 2b065f8c3f71..3c54e95964c9 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -145,7 +145,6 @@ static void *z_erofs_lz4_handle_overlap(const struct z_erofs_decompress_req *rq,
 	oend = rq->pageofs_out + rq->outputsize;
 	omargin = PAGE_ALIGN(oend) - oend;
 	if (!rq->partial_decoding && may_inplace &&
-	    rq->outpages >= rq->inpages &&
 	    omargin >= LZ4_DECOMPRESS_INPLACE_MARGIN(rq->inputsize)) {
 		for (i = 0; i < rq->inpages; ++i)
 			if (rq->out[rq->outpages - rq->inpages + i] !=
diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 67f55b9b57af..72b96e295716 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -473,11 +473,6 @@ static int z_erofs_map_blocks_fo(struct inode *inode,
 	}
 
 	if (m.headtype == Z_EROFS_LCLUSTER_TYPE_PLAIN) {
-		if (map->m_llen > map->m_plen) {
-			DBG_BUGON(1);
-			err = -EFSCORRUPTED;
-			goto unmap_out;
-		}
 		if (vi->z_advise & Z_EROFS_ADVISE_INTERLACED_PCLUSTER)
 			map->m_algorithmformat = Z_EROFS_COMPRESSION_INTERLACED;
 		else
@@ -720,10 +715,21 @@ static int z_erofs_map_sanity_check(struct inode *inode,
 			  map->m_algorithmformat, map->m_la, EROFS_I(inode)->nid);
 		return -EOPNOTSUPP;
 	}
-	if (unlikely(map->m_algorithmformat < Z_EROFS_COMPRESSION_MAX &&
-		     !(sbi->available_compr_algs & (1 << map->m_algorithmformat)))) {
-		erofs_err(inode->i_sb, "inconsistent algorithmtype %u for nid %llu",
-			  map->m_algorithmformat, EROFS_I(inode)->nid);
+
+	if (map->m_algorithmformat < Z_EROFS_COMPRESSION_MAX) {
+		if (sbi->available_compr_algs ^ BIT(map->m_algorithmformat)) {
+			erofs_err(inode->i_sb, "inconsistent algorithmtype %u for nid %llu",
+				  map->m_algorithmformat, EROFS_I(inode)->nid);
+			return -EFSCORRUPTED;
+		}
+		if (EROFS_MAP_FULL(map->m_flags) && map->m_llen < map->m_plen) {
+			erofs_err(inode->i_sb, "too much compressed data @ la %llu of nid %llu",
+				  map->m_la, EROFS_I(inode)->nid);
+			return -EFSCORRUPTED;
+		}
+	} else if (map->m_llen > map->m_plen) {
+		erofs_err(inode->i_sb, "not enough plain data on disk @ la %llu of nid %llu",
+			  map->m_la, EROFS_I(inode)->nid);
 		return -EFSCORRUPTED;
 	}
 	if (unlikely(map->m_plen > Z_EROFS_PCLUSTER_MAX_SIZE ||
-- 
2.43.5