[PATCH] jffs2: reject uncompressed nodes whose csize and dsize differ

[PATCH] jffs2: reject uncompressed nodes whose csize and dsize differ

[Alejandro Pacheco]

From: Alejandro Pacheco <ale@alepacheco.dev>

jffs2_read_dnode() takes the length of an uncompressed
(JFFS2_COMPR_NONE) node's data from its on-media csize, but sizes and
bounds the copy by its dsize. Nothing checks that the two agree:

 - reading the whole node, it reads csize bytes straight into the
   caller's buffer, which holds dsize bytes, so a csize larger than
   dsize writes past the end of that buffer;
 - reading part of the node, it reads csize bytes into a csize-sized
   buffer and then copies len bytes from offset ofs of it, checking
   only that ofs + len fits in dsize, so a csize smaller than dsize
   lets the copy read past the end of that buffer.

The check that ofs + len lies within dsize is also only built when
debug messages are enabled (D1()).

For an uncompressed node csize and dsize must be equal: every writer
in the kernel, and mkfs.jffs2, writes them so. Check that, and make
the ofs + len range check unconditional, in jffs2_read_dnode(),
returning -EIO with a warning, as it does for a data CRC mismatch.
read_dnode() also drops such a node when it first checks it, as it
does a node whose totlen is wrong, so the node never enters the
inode's fragment tree.

It was tested on a generic KASAN UML kernel built from this tree, with
jffs2 on a 4 MiB mtdram device. Files written uncompressed (compr=none)
and zlib-compressed were partly overwritten, so that later reads take
part of a node, and a device node and a symlink were made, so that
whole metadata nodes are read. All of them read back correctly after
remount, after setattr and remount, and after enough writes that the
garbage collector had to erase and rewrite the device, with no KASAN
report and no warning in the kernel log. This runs the new checks on
valid nodes only: no image with mismatched sizes was made. It builds
with no new warnings (W=1, x86_64 defconfig with JFFS2 and KASAN, with
and without CONFIG_JFFS2_FS_DEBUG), and checkpatch.pl --strict reports
nothing.

An LLM coding assistant was used to find this bug and to write this
patch and its changelog.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Alejandro Pacheco <ale@alepacheco.dev>
---
 fs/jffs2/read.c      | 20 +++++++++++++++-----
 fs/jffs2/readinode.c |  8 ++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/fs/jffs2/read.c b/fs/jffs2/read.c
index 0b042b1fc82..0de9e612ed5 100644
--- a/fs/jffs2/read.c
+++ b/fs/jffs2/read.c
@@ -68,13 +68,23 @@ int jffs2_read_dnode(struct jffs2_sb_info *c, struct jffs2_inode_info *f,
 		ri->csize = cpu_to_je32(0);
 	}
 
-	D1(if(ofs + len > je32_to_cpu(ri->dsize)) {
-			pr_warn("jffs2_read_dnode() asked for %d bytes at %d from %d-byte node\n",
-				len, ofs, je32_to_cpu(ri->dsize));
-		ret = -EINVAL;
+	if (ofs + len > je32_to_cpu(ri->dsize)) {
+		pr_warn("%s(): asked for %d bytes at %d from %u-byte node at 0x%08x\n",
+			__func__, len, ofs, je32_to_cpu(ri->dsize),
+			ref_offset(fd->raw));
+		ret = -EIO;
 		goto out_ri;
-	});
+	}
 
+	/* Uncompressed data is copied straight out of a csize buffer */
+	if (ri->compr == JFFS2_COMPR_NONE &&
+	    je32_to_cpu(ri->csize) != je32_to_cpu(ri->dsize)) {
+		pr_warn("Uncompressed node at 0x%08x has csize 0x%x != dsize 0x%x\n",
+			ref_offset(fd->raw), je32_to_cpu(ri->csize),
+			je32_to_cpu(ri->dsize));
+		ret = -EIO;
+		goto out_ri;
+	}
 
 	if (ri->compr == JFFS2_COMPR_ZERO) {
 		memset(buf, 0, len);
diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c
index 1caabff9dc9..702418abfa7 100644
--- a/fs/jffs2/readinode.c
+++ b/fs/jffs2/readinode.c
@@ -751,6 +751,14 @@ static inline int read_dnode(struct jffs2_sb_info *c, struct jffs2_raw_node_ref
 			goto free_out;
 		}
 
+		if (unlikely(rd->compr == JFFS2_COMPR_NONE &&
+			     csize != je32_to_cpu(rd->dsize))) {
+			JFFS2_WARNING("uncompressed node at %#08x has csize %#x != dsize %#x\n",
+				      ref_offset(ref), csize, je32_to_cpu(rd->dsize));
+			jffs2_mark_node_obsolete(c, ref);
+			goto free_out;
+		}
+
 		if (jffs2_is_writebuffered(c) && csize != 0) {
 			/* At this point we are supposed to check the data CRC
 			 * of our unchecked node. But thus far, we do not