[PATCH] jffs2: validate summary offset in the XIP scan path

[PATCH] jffs2: validate summary offset in the XIP scan path

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

jffs2_scan_eraseblock() locates the summary node at the end of an
eraseblock and derives both a pointer and a length from the on-flash
summary marker:

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

In the non-XIP branch this is followed by

	if (sumlen > c->sector_size)
		goto full_scan;

so a bogus sm->offset (for which sector_size - offset underflows to a
large value) is rejected.  The XIP branch (taken when the whole device
was successfully mtd_point()ed, e.g. an mtdram-backed MTD) is missing
that check.

On a crafted image an attacker controls the bytes of the summary marker
at the end of the last eraseblock.  Setting sm->magic == JFFS2_SUM_MAGIC
and sm->offset > sector_size makes sumlen wrap to ~4 GiB while sumptr
still points inside the mapped flash.  jffs2_sum_scan_sumnode() then runs

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

with the attacker-controlled sumsize, reading far past the end of the
mtd_point() mapping -- an out-of-bounds read (KASAN: vmalloc OOB /
general protection fault) of controlled length.  The preceding header
CRC checks are computed over data read from the attacker-controlled
flash and can be satisfied, so they do not prevent the over-read.

Mirror the non-XIP guard in the XIP branch.

Reproduced under KASAN on 7.4-rc1 by mounting a crafted mtdram-backed
image (the XIP mtd_point() branch): KASAN reports a vmalloc-out-of-bounds
read in the summary-node crc32 over the mapped flash.

Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 fs/jffs2/scan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -495,6 +495,10 @@ static int jffs2_scan_eraseblock (struct jffs2_sb_info *c, struct jffs2_eraseblo
 			if (je32_to_cpu(sm->magic) == JFFS2_SUM_MAGIC) {
 				sumptr = buf + je32_to_cpu(sm->offset);
 				sumlen = c->sector_size - je32_to_cpu(sm->offset);
+
+				/* sm->offset may be garbage even if MAGIC matched */
+				if (sumlen > c->sector_size)
+					goto full_scan;
 			}
 		} else {
 			/* If NAND flash, read a whole page of it. Else just the end */

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

[hyunjungg]

From: Hyunjung Ko <hj351016@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@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",