Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 8 May 2023 16:35:15 -0400
From:   "Theodore Ts'o" <tytso@mit.edu>
To:     Jan Kara <jack@suse.cz>
Cc:     Ext4 Developers List <linux-ext4@vger.kernel.org>,
        syzbot+e2efa3efc15a1c9e95c3@syzkaller.appspotmail.com
Subject: Re: [PATCH 2/2] ext4: remove a BUG_ON in ext4_mb_release_group_pa()
Message-ID: <ZFldA72PlRBDOrRD@mit.edu>
References: <20230430154311.579720-1-tytso@mit.edu>
 <20230430154311.579720-3-tytso@mit.edu>
 <20230507182833.ma7fugevh7imz2tj@quack3>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230507182833.ma7fugevh7imz2tj@quack3>
Precedence: bulk

On Sun, May 07, 2023 at 08:28:33PM +0200, Jan Kara wrote:
> OK, looks good to me. But frankly there are many other interesting ways how
> bogus group numbers output when this happens can return is fun stuff - e.g.
> ext4_group_first_block_no() is going to return invalid blocks etc. So it
> feels a bit like endless whack-a-mole game. Anyway I agree the series seem
> to fix a big chunk of these sites so feel free to add:

The main thing I'm trying to avoid is a kernel crash or possible
out-of-bounds read or write, which could lead to a security
vulnerability.  If a a bad block number being returned by. say,
ext4_group_first_block_no() "only" results in an I/O error when or
(further) corruption of the block device, that's not a problem as far
as I'm concerned.  After all, if a malicious root access has
read/write access to the block device, they can corrupt the file
system *anyway*.

I wasn't able to find cases where a crazy return value from
ext4_group_first_block_no() which would cause a BUG or a buffer
overrun.  If we (or syzbot) finds a case where this could happen, we
could copy s_es->s_first_data_block to sbi->s_first_data_block and
then validate it during the file system mount.

I also did a quick spot check what nastiness could be caused by
real-time frobbing of s_blocks_count s_inodes_count and couldn't find
anything there either.  So it looks like s_first_data_block is the one
which is the most problematic.

Cheers,

						- Ted