linux-kernel - [PATCH] isofs: fix Oops with zisofs and large PAGE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20201120140633.1673-1-scdbackup@gmx.net>
Date:   Fri, 20 Nov 2020 15:06:33 +0100
From:   Thomas Schmitt <scdbackup@....net>
To:     linux-scsi@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, arnd@...db.de, axboe@...nel.dk,
        hpa@...or.com, debian-powerpc@...ts.debian.org,
        Thomas Schmitt <scdbackup@....net>,
        Anatoly Pugachev <matorola@...il.com>
Subject: [PATCH] isofs: fix Oops with zisofs and large PAGE_SIZE

On a "5.9.0-1-powerpc64" system with PAGE_SIZE 64 KiB, the attempt to read
a zisofs compressed file with zisofs block size 32 KiB yields a kernel
Oops if the file contains sparse blocks of 32 KiB all zeros.

BUG: Unable to handle kernel data access on read at 0xfff01417f3f98000
Faulting instruction address: 0xc0000000000ad4b0
Oops: Kernel access of bad area, sig: 11 [#1]
BE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: isofs(E) loop(E) sg(E) drm(E)
  drm_panel_orientation_quirks(E) ip_tables(E) x_tables(E)
  autofs4(E) ext4(E) crc16(E) mb>
CPU: 0 PID: 12433 Comm: md5sum Tainted: G            E
  5.9.0-1-powerpc64 #1 Debian 5.9.1-1
NIP:  c0000000000ad4b0 LR: c008000001086628 CTR: 0000000000000200
[... lines "REGS:" to "GPR28:" skipped ...]
NIP [c0000000000ad4b0] .memset+0x68/0x104
LR [c008000001086628]  .zisofs_readpage+0xbd8/0xd90 [isofs]
Call Trace:
[c0000003f40fb650] [c008000001085ec0] .zisofs_readpage+0x470/0xd90 [isofs]
(unreliable)
[c0000003f40fb830] [c0000000003a4d08] .read_pages+0x2f8/0x390
[c0000003f40fb900] [c0000000003a53b4] .page_cache_readahead_unbounded
+0x224/0x2f0
[c0000003f40fba20] [c0000000003960c4] .generic_file_buffered_read
+0x704/0xcf0
[...]

Userland experiences a segmentation fault.

This happened when the first 32 KiB of the original uncompressed file were
not all 0, but the next 32 KiB were all 0.

Cause is the handling of sparse compression blocks in function
zisofs_uncompress_block(). It does does not take into account that the
compression block contains less data than the page can take, but rather
zeros the whole page and returns as decompressed data amount the PAGE_SIZE
instead of the zisofs block size.
This causes the wrong impression with the caller zisofs_fill_pages()
that an incomplete page should be padded up:

        if (poffset && *pages) {
                memset(page_address(*pages) + poffset, 0,
                       PAGE_SIZE - poffset);

poffset is 32 KiB from the wrong decompressed amount of 32K + 64K instead
of the correct 32K + 32K, which would have yielded poffset 0.
But at that time the pages pointer has already moved beyond the end of
the allocated array of page structs (which has only 1 element).

Signed-off-by: Thomas Schmitt <scdbackup@....net>
Tested-by: Anatoly Pugachev <matorola@...il.com>
---
 fs/isofs/compress.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/fs/isofs/compress.c b/fs/isofs/compress.c
index bc12ac7e2312..15a5b4e63f97 100644
--- a/fs/isofs/compress.c
+++ b/fs/isofs/compress.c
@@ -43,6 +43,7 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
 				      int *errp)
 {
 	unsigned int zisofs_block_shift = ISOFS_I(inode)->i_format_parm[1];
+	unsigned long zisofs_block_size = 1UL << zisofs_block_shift;
 	unsigned int bufsize = ISOFS_BUFFER_SIZE(inode);
 	unsigned int bufshift = ISOFS_BUFFER_BITS(inode);
 	unsigned int bufmask = bufsize - 1;
@@ -57,21 +58,39 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
 	blkcnt_t blocknum;
 	struct buffer_head **bhs;
 	int curbh, curpage;
+	loff_t zeros_count = 0;
+	unsigned int zeros_chunk;

-	if (block_size > deflateBound(1UL << zisofs_block_shift)) {
+	if (block_size > deflateBound(zisofs_block_size)) {
 		*errp = -EIO;
 		return 0;
 	}
 	/* Empty block? */
 	if (block_size == 0) {
+		/*
+		 * If poffset is > 0 take care to fill only a part of pages[0].
+		 * This is supposed to happen only if
+		 *   zisofs_block_size < PAGE_SIZE
+		 * pcount is then supposed to be 1, but better enforce the
+		 * loop end after the first pass.
+		 */
 		for ( i = 0 ; i < pcount ; i++ ) {
-			if (!pages[i])
-				continue;
-			memset(page_address(pages[i]), 0, PAGE_SIZE);
-			flush_dcache_page(pages[i]);
-			SetPageUptodate(pages[i]);
+			zeros_chunk = min_t(unsigned long, PAGE_SIZE - poffset,
+					    zisofs_block_size);
+			zeros_count += zeros_chunk;
+			if (pages[i]) {
+				memset(page_address(pages[i]) + poffset, 0,
+				       zeros_chunk);
+				if (poffset + zeros_chunk >= PAGE_SIZE) {
+					flush_dcache_page(pages[i]);
+					SetPageUptodate(pages[i]);
+				}
+			}
+			poffset = 0;
+			if (zisofs_block_size < PAGE_SIZE)
+				break;
 		}
-		return ((loff_t)pcount) << PAGE_SHIFT;
+		return zeros_count;
 	}

 	/* Because zlib is not thread-safe, do all the I/O at the top. */
--
2.20.1