Forwarded: [PATCH] ext4: fix general protection fault in bio_add_page for encrypted large folios

syzbot posted 1 patch 1 week, 6 days ago
fs/ext4/page-io.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
Forwarded: [PATCH] ext4: fix general protection fault in bio_add_page for encrypted large folios
Posted by syzbot 1 week, 6 days ago
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] ext4: fix general protection fault in bio_add_page for encrypted large folios
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master


When writing back an encrypted file, ext4_bio_write_folio() encrypts the
folio into a single-page bounce buffer and passes it as io_folio to
io_submit_add_bh(). The offset passed to bio_add_folio() was always
bh_offset(bh), which is relative to the original folio.

For a large folio this offset can exceed PAGE_SIZE. bio_add_folio() calls
folio_page(io_folio, off >> PAGE_SHIFT) which computes &folio->page + N.
For a single-page bounce folio with N >= 1 this is out-of-bounds, causing
a general protection fault caught by KASAN:

  KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
  RIP: 0010:bvec_set_page include/linux/bvec.h:44 [inline]
  RIP: 0010:bio_add_page+0x462/0x6e0 block/bio.c:1048

Fix this by computing io_off at the call site. For the non-encrypted path
io_folio == folio so bh_offset(bh) is used unchanged. For the encrypted
path the bounce page is always a single PAGE_SIZE page, so the offset is
taken modulo PAGE_SIZE to map it correctly into the bounce page.

Using hardcoded 0 would be wrong for sub-page block sizes (e.g. 1024-byte
blocks) where multiple buffer heads exist within one page at offsets
0, 1024, 2048, 3072 etc. bh_offset(bh) % PAGE_SIZE handles all block
sizes correctly.

Reported-by: syzbot+ed8bc247f231c1a48e21@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ed8bc247f231c1a48e21
Signed-off-by: Deepanshu Kartikey <Kartikey406@gmail.com>
---
 fs/ext4/page-io.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index a8c95eee91b7..006b2f5173de 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -438,7 +438,8 @@ static void io_submit_add_bh(struct ext4_io_submit *io,
 			     struct inode *inode,
 			     struct folio *folio,
 			     struct folio *io_folio,
-			     struct buffer_head *bh)
+			     struct buffer_head *bh,
+		             size_t io_off)
 {
 	if (io->io_bio && (bh->b_blocknr != io->io_next_block ||
 			   !fscrypt_mergeable_bio_bh(io->io_bio, bh))) {
@@ -449,7 +450,7 @@ static void io_submit_add_bh(struct ext4_io_submit *io,
 		io_submit_init_bio(io, bh);
 		io->io_bio->bi_write_hint = inode->i_write_hint;
 	}
-	if (!bio_add_folio(io->io_bio, io_folio, bh->b_size, bh_offset(bh)))
+	if (!bio_add_folio(io->io_bio, io_folio, bh->b_size, io_off))
 		goto submit_and_retry;
 	wbc_account_cgroup_owner(io->io_wbc, folio, bh->b_size);
 	io->io_next_block++;
@@ -585,9 +586,20 @@ int ext4_bio_write_folio(struct ext4_io_submit *io, struct folio *folio,
 
 	/* Now submit buffers to write */
 	do {
+		size_t io_off;
+
 		if (!buffer_async_write(bh))
 			continue;
-		io_submit_add_bh(io, inode, folio, io_folio, bh);
+		/*
+		 * When io_folio is a single-page bounce buffer (fscrypt),
+		 * normalise to PAGE_SIZE to handle all block sizes correctly.
+		 * Using 0 would break sub-page block sizes (e.g. 1024-byte
+		 * blocks) where multiple bh offsets exist within one page
+		 */
+		io_off = (io_folio == folio)
+			 ? bh_offset(bh)
+			 : bh_offset(bh) % PAGE_SIZE;
+		io_submit_add_bh(io, inode, folio, io_folio, bh, io_off);
 	} while ((bh = bh->b_this_page) != head);
 
 	return 0;
-- 
2.43.0