[PATCH v2] jffs2: validate summary length before scanning the summary node

[hyunjungg]

From: Hyunjung Ko <hj351016 at gmail.com>

jffs2_scan_eraseblock() locates the summary node at the end of an
eraseblock and derives its length from an on-flash offset in the
summary marker:

	sumptr = buf + je32_to_cpu(sm->offset);
	sumlen = c->sector_size - je32_to_cpu(sm->offset);

On a crafted image the summary marker bytes at the end of the last
eraseblock are attacker-controlled, and two malformed values slip
through:

  - sm->offset > sector_size makes sumlen underflow to a huge value.
    The non-XIP branch rejects this with "sumlen > c->sector_size", but
    the XIP branch (whole device mtd_point()ed, e.g. an mtdram-backed
    MTD) has no such check.

  - sm->offset within the last few bytes of the eraseblock makes sumlen
    small (0 .. sizeof(struct jffs2_raw_summary) - 1); this passes the
    upper-bound check in both branches.

jffs2_sum_scan_sumnode() then dereferences the pointer as a
struct jffs2_raw_summary to read its fixed header and computes

	crc32(0, summary->sum, sumsize - sizeof(struct jffs2_raw_summary));

Without a lower bound the header read runs past the buffer (or past the
mtd_point() mapping in the XIP case), and for a too-small sumsize the
crc32 length underflows to ~4 GiB -> an out-of-bounds read of
attacker-controlled length (KASAN: vmalloc-out-of-bounds, or a general
protection fault on a real flash mapping). The summary header CRCs are
computed over the attacker-controlled flash and can be satisfied, so
they do not prevent the over-read.

jffs2_sum_scan_sumnode() is the sole consumer of this length (it is
only called from jffs2_scan_eraseblock()), so bound it there once -
covering both the XIP and non-XIP paths and both the underflow and the
too-small cases: reject a sumsize that cannot hold the fixed header or
that exceeds the eraseblock, and fall back to a full scan.

Reproduced under KASAN on 7.4-rc1 by mounting a crafted mtdram-backed
image whose summary marker sets sm->offset > sector_size (the XIP
path): KASAN reports a vmalloc-out-of-bounds read in the summary-node
crc32. The small-sumsize case is the same over-read and length
underflow in the same function.

Fixes: 9641b784ff82 ("[JFFS2] Optimise reading of eraseblock summary nodes")
Signed-off-by: Hyunjung Ko <hj351016 at gmail.com>
---
v2:
 - Move the check into jffs2_sum_scan_sumnode() instead of only the XIP
   branch of jffs2_scan_eraseblock(), so both the XIP and non-XIP paths
   are covered in one place (per Sashiko AI review).
 - Also reject sumsize < sizeof(struct jffs2_raw_summary): a summary
   offset within the last few bytes of the eraseblock yields a small
   sumlen that passed the previous upper-bound-only check but still
   over-reads the header and underflows the crc32 length (per review).
 - Add Fixes: tag.

 fs/jffs2/summary.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/fs/jffs2/summary.c b/fs/jffs2/summary.c
--- a/fs/jffs2/summary.c
+++ b/fs/jffs2/summary.c
@@ -577,6 +577,19 @@ int jffs2_sum_scan_sumnode(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb
 	int ret, ofs;
 	uint32_t crc;

+	/*
+	 * sumsize comes from an on-flash offset in the summary marker and
+	 * is attacker-controlled on a crafted image.  It must be large
+	 * enough to hold the fixed summary header and must not exceed the
+	 * eraseblock; otherwise the header accesses below read past the
+	 * buffer (or the mtd_point() mapping in the XIP case) and the
+	 * "sumsize - sizeof(struct jffs2_raw_summary)" crc32 length
+	 * underflows.  Fall back to a full scan in that case.
+	 */
+	if (sumsize < sizeof(struct jffs2_raw_summary) ||
+	    sumsize > c->sector_size)
+		return 0;
+
 	ofs = c->sector_size - sumsize;

 	dbg_summary("summary found for 0x%08x at 0x%08x (0x%x bytes)\n",