[PATCH] jffs2: return -EIO when a decompressor reports failure

[PATCH] jffs2: return -EIO when a decompressor reports failure

[Karthikeyan KS]

jffs2_decompress() passes the decompressor's return value straight up.
The rtime and zlib decompressors report failure by returning a positive
1. That breaks the kernel's error convention: a function that can fail
returns 0 or a negative errno, and generic code (the page cache, the
write path, IS_ERR()) relies on that, so a positive value is read as
success. JFFS2's own callers only check for non-zero, which hides the
violation: the value travels unchanged through jffs2_read_dnode(),
jffs2_read_inode_range() and jffs2_do_readpage_nolock() to that generic
code.

On a read, filemap_get_pages() returns 1 without filling its folio
batch, filemap_read() carries on and dereferences the empty batch:

  jffs2: Decompressor "rtime" returned 1
  jffs2: Error: jffs2_decompress returned 1
  Unhandled fault: page domain fault (0x01b) at 0x00000000
  PC is at filemap_read+0x1d8/0x5b4
  Call trace:
   filemap_read from vfs_read+0x23c/0x318
   vfs_read from ksys_read+0x74/0xf0
   ksys_read from ret_fast_syscall+0x0/0x54
  Kernel panic - not syncing: Fatal exception

On a partial write, jffs2_write_begin() has to read the old page in.
When that fails it returns 1, generic_perform_write() carries on, and
jffs2_write_end() is called for a folio that is not uptodate:

  jffs2: Error: jffs2_decompress returned 1
  kernel BUG at fs/jffs2/file.c:270!
  PC is at jffs2_write_end+0x50/0x3b4
  Call trace:
   jffs2_write_end from generic_perform_write+0x144/0x280
   generic_perform_write from generic_file_write_iter+0x64/0x10c
   generic_file_write_iter from vfs_write+0x230/0x3e8
   vfs_write from sys_pwrite64+0xa0/0xc8

Garbage collection has the same problem. read_cache_folio() returns
ERR_PTR(1), which IS_ERR() does not treat as an error, and the "folio"
at address 0x1 gets mapped:

  jffs2: Error: jffs2_decompress returned 1
  Unhandled fault: page domain fault (0x01b) at 0x00000001
  PC is at __kmap_local_page_prot+0x8/0x5c
  LR is at jffs2_garbage_collect_dnode+0x1dc/0x518
  Call trace:
   __kmap_local_page_prot from jffs2_garbage_collect_dnode+0x1dc/0x518
   jffs2_garbage_collect_dnode from jffs2_garbage_collect_pass+0xa54/0xec4
   jffs2_garbage_collect_pass from jffs2_reserve_space+0x12c/0x344

Restore the convention at the one place every decompressor result
passes through: convert any positive return value into -EIO in
jffs2_decompress(), the only caller of the decompressors, so that its
callers get a negative errno. The warning still shows the value the
decompressor returned. Negative return values (lzo returns -1) are not
changed.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: fe051552f507 ("jffs2: Prevent rtime decompress memory corruption")
Cc: stable@vger.kernel.org
Signed-off-by: Karthikeyan KS <karthiproffesional@gmail.com>
---

Hi Richard,

How this was found: we backported fe051552f507 ("jffs2: Prevent rtime
decompress memory corruption") to our kernel without b29bf7119d6b
("jffs2: Fix rtime decompressor"). On an AST2500 SoC, the off-by-one in
the rtime bounds check then rejected a valid node. We reproduced the
resulting failure paths on qemu, using crafted flash images that make an
rtime or zlib node fail to decompress.

Tested on: qemu ast2500-evb (aspeed_g5_defconfig, v7.3-rc7, 32-bit ARM)
with images holding an rtime or zlib node that fails to decompress. With
the patch, read() fails with EIO, a mmap fault raises SIGBUS, pwrite()
fails with EIO, GC logs "read_cache_folio() returned error: -5", and
nothing oopses. Valid rtime, zlib, lzo and uncompressed files read and
write the same as before. We did not test GC with zlib or lzo nodes, or
on real hardware.

 fs/jffs2/compr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/jffs2/compr.c b/fs/jffs2/compr.c
index 764f19dec3f0..da317ed3ccdb 100644
--- a/fs/jffs2/compr.c
+++ b/fs/jffs2/compr.c
@@ -288,6 +288,8 @@ int jffs2_decompress(struct jffs2_sb_info *c, struct jffs2_inode_info *f,
 				}
 				this->usecount--;
 				spin_unlock(&jffs2_compressor_list_lock);
+				if (ret > 0)
+					ret = -EIO;
 				return ret;
 			}
 		}
--
2.43.0