linux-kernel - Re: [PATCH] bcachefs: fix oob in bch2_sb_clean_to

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <x73mcz4gjzdmwlynj426sq34zj232wr2z2xjbjc4tkcdbfpegb@hr55sh6pn7ol>
Date: Tue, 7 May 2024 10:14:22 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Edward Adam Davis <eadavis@...com>
Cc: syzbot+c48865e11e7e893ec4ab@...kaller.appspotmail.com, 
	bfoster@...hat.com, linux-bcachefs@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH] bcachefs: fix oob in bch2_sb_clean_to_text

On Tue, May 07, 2024 at 05:19:29PM +0800, Edward Adam Davis wrote:
> When got too small clean field, entry will never equal vstruct_end(&clean->field), 
> the dead loop resulted in out of bounds access.
> 
> Fixes: 12bf93a429c9 ("bcachefs: Add .to_text() methods for all superblock sections")
> Fixes: a37ad1a3aba9 ("bcachefs: sb-clean.c")
> Reported-and-tested-by: syzbot+c48865e11e7e893ec4ab@...kaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@...com>

I've already got a patch up for this - the validation was missing as
well.

commit f39055220f6f98a180e3503fe05bbf9921c425c8
Author: Kent Overstreet <kent.overstreet@...ux.dev>
Date:   Sun May 5 22:28:00 2024 -0400

    bcachefs: Add missing validation for superblock section clean
    
    We were forgetting to check for jset entries that overrun the end of the
    section - both in validate and to_text(); to_text() needs to be safe for
    types that fail to validate.
    
    Reported-by: syzbot+c48865e11e7e893ec4ab@...kaller.appspotmail.com
    Signed-off-by: Kent Overstreet <kent.overstreet@...ux.dev>

diff --git a/fs/bcachefs/sb-clean.c b/fs/bcachefs/sb-clean.c
index 35ca3f138de6..194e55b11137 100644
--- a/fs/bcachefs/sb-clean.c
+++ b/fs/bcachefs/sb-clean.c
@@ -278,6 +278,17 @@ static int bch2_sb_clean_validate(struct bch_sb *sb,
 		return -BCH_ERR_invalid_sb_clean;
 	}
 
+	for (struct jset_entry *entry = clean->start;
+	     entry != vstruct_end(&clean->field);
+	     entry = vstruct_next(entry)) {
+		if ((void *) vstruct_next(entry) > vstruct_end(&clean->field)) {
+			prt_str(err, "entry type ");
+			bch2_prt_jset_entry_type(err, le16_to_cpu(entry->type));
+			prt_str(err, " overruns end of section");
+			return -BCH_ERR_invalid_sb_clean;
+		}
+	}
+
 	return 0;
 }
 
@@ -295,6 +306,9 @@ static void bch2_sb_clean_to_text(struct printbuf *out, struct bch_sb *sb,
 	for (entry = clean->start;
 	     entry != vstruct_end(&clean->field);
 	     entry = vstruct_next(entry)) {
+		if ((void *) vstruct_next(entry) > vstruct_end(&clean->field))
+			break;
+
 		if (entry->type == BCH_JSET_ENTRY_btree_keys &&
 		    !entry->u64s)
 			continue;