Re: CVE-2024-41067: btrfs: scrub: handle RST lookup error correctly

Re: CVE-2024-41067: btrfs: scrub: handle RST lookup error correctly

[David Sterba]

On Mon, Jul 29, 2024 at 04:58:13PM +0200, Greg Kroah-Hartman wrote:
> Description
> ===========
> 
> In the Linux kernel, the following vulnerability has been resolved:
> 
> btrfs: scrub: handle RST lookup error correctly
> 
> [BUG]
> When running btrfs/060 with forced RST feature, it would crash the
> following ASSERT() inside scrub_read_endio():
> 
> 	ASSERT(sector_nr < stripe->nr_sectors);
> 
> Before that, we would have tree dump from
> btrfs_get_raid_extent_offset(), as we failed to find the RST entry for
> the range.
> 
> [CAUSE]
> Inside scrub_submit_extent_sector_read() every time we allocated a new
> bbio we immediately called btrfs_map_block() to make sure there was some
> RST range covering the scrub target.
> 
> But if btrfs_map_block() fails, we immediately call endio for the bbio,
> while the bbio is newly allocated, it's completely empty.
> 
> Then inside scrub_read_endio(), we go through the bvecs to find
> the sector number (as bi_sector is no longer reliable if the bio is
> submitted to lower layers).
> 
> And since the bio is empty, such bvecs iteration would not find any
> sector matching the sector, and return sector_nr == stripe->nr_sectors,
> triggering the ASSERT().
> 
> [FIX]
> Instead of calling btrfs_map_block() after allocating a new bbio, call
> btrfs_map_block() first.
> 
> Since our only objective of calling btrfs_map_block() is only to update
> stripe_len, there is really no need to do that after btrfs_alloc_bio().
> 
> This new timing would avoid the problem of handling empty bbio
> completely, and in fact fixes a possible race window for the old code,
> where if the submission thread is the only owner of the pending_io, the
> scrub would never finish (since we didn't decrease the pending_io
> counter).
> 
> Although the root cause of RST lookup failure still needs to be
> addressed.
> 
> The Linux kernel CVE team has assigned CVE-2024-41067 to this issue.

Please drop the CVE. It's a fix for feature that's still in development
and is not enabled on production kernels (requires CONFIG_BTRFS_DEBUG).
There was even a recent on-disk format change (mkfs required), this is
not really for environments where security matters. Thanks.

Re: CVE-2024-41067: btrfs: scrub: handle RST lookup error correctly

[Greg Kroah-Hartman]

On Mon, Jul 29, 2024 at 10:55:03PM +0200, David Sterba wrote:
> On Mon, Jul 29, 2024 at 04:58:13PM +0200, Greg Kroah-Hartman wrote:
> > Description
> > ===========
> > 
> > In the Linux kernel, the following vulnerability has been resolved:
> > 
> > btrfs: scrub: handle RST lookup error correctly
> > 
> > [BUG]
> > When running btrfs/060 with forced RST feature, it would crash the
> > following ASSERT() inside scrub_read_endio():
> > 
> > 	ASSERT(sector_nr < stripe->nr_sectors);
> > 
> > Before that, we would have tree dump from
> > btrfs_get_raid_extent_offset(), as we failed to find the RST entry for
> > the range.
> > 
> > [CAUSE]
> > Inside scrub_submit_extent_sector_read() every time we allocated a new
> > bbio we immediately called btrfs_map_block() to make sure there was some
> > RST range covering the scrub target.
> > 
> > But if btrfs_map_block() fails, we immediately call endio for the bbio,
> > while the bbio is newly allocated, it's completely empty.
> > 
> > Then inside scrub_read_endio(), we go through the bvecs to find
> > the sector number (as bi_sector is no longer reliable if the bio is
> > submitted to lower layers).
> > 
> > And since the bio is empty, such bvecs iteration would not find any
> > sector matching the sector, and return sector_nr == stripe->nr_sectors,
> > triggering the ASSERT().
> > 
> > [FIX]
> > Instead of calling btrfs_map_block() after allocating a new bbio, call
> > btrfs_map_block() first.
> > 
> > Since our only objective of calling btrfs_map_block() is only to update
> > stripe_len, there is really no need to do that after btrfs_alloc_bio().
> > 
> > This new timing would avoid the problem of handling empty bbio
> > completely, and in fact fixes a possible race window for the old code,
> > where if the submission thread is the only owner of the pending_io, the
> > scrub would never finish (since we didn't decrease the pending_io
> > counter).
> > 
> > Although the root cause of RST lookup failure still needs to be
> > addressed.
> > 
> > The Linux kernel CVE team has assigned CVE-2024-41067 to this issue.
> 
> Please drop the CVE. It's a fix for feature that's still in development
> and is not enabled on production kernels (requires CONFIG_BTRFS_DEBUG).

We do not know people's use case, and can not "gate" CVE ids based on
difference config options like this.

> There was even a recent on-disk format change (mkfs required), this is
> not really for environments where security matters. Thanks.

It's a fix for a vulnerability, so I think it should stay assigned.  If
your system does not enable that config option, then there is nothing to
worry about, right?

thanks,

greg k-h

Re: CVE-2024-41067: btrfs: scrub: handle RST lookup error correctly

[David Sterba]

On Tue, Jul 30, 2024 at 06:53:40AM +0200, Greg Kroah-Hartman wrote:
> > > The Linux kernel CVE team has assigned CVE-2024-41067 to this issue.
> > 
> > Please drop the CVE. It's a fix for feature that's still in development
> > and is not enabled on production kernels (requires CONFIG_BTRFS_DEBUG).
> 
> We do not know people's use case, and can not "gate" CVE ids based on
> difference config options like this.

The use case are early adopters for a known unfinished feature where
instabilty could happen and there are known problems like lockups.  It's
behind the config option for that purpose so bugs are reported but don't
have to be treated as security.

> > There was even a recent on-disk format change (mkfs required), this is
> > not really for environments where security matters. Thanks.
> 
> It's a fix for a vulnerability, so I think it should stay assigned.  If
> your system does not enable that config option, then there is nothing to
> worry about, right?

It's not a vulnerability unless you define that very extensively. My
systems don't enable that, distro kernels don't enable that, so there's
nothing to worry about. Conclusiion is not to assign a CVE, right.
Otherwise it only increases paperwork.

Re: CVE-2024-41067: btrfs: scrub: handle RST lookup error correctly

[Greg Kroah-Hartman]

On Wed, Jul 31, 2024 at 04:34:42PM +0200, David Sterba wrote:
> On Tue, Jul 30, 2024 at 06:53:40AM +0200, Greg Kroah-Hartman wrote:
> > > > The Linux kernel CVE team has assigned CVE-2024-41067 to this issue.
> > > 
> > > Please drop the CVE. It's a fix for feature that's still in development
> > > and is not enabled on production kernels (requires CONFIG_BTRFS_DEBUG).
> > 
> > We do not know people's use case, and can not "gate" CVE ids based on
> > difference config options like this.
> 
> The use case are early adopters for a known unfinished feature where
> instabilty could happen and there are known problems like lockups.  It's
> behind the config option for that purpose so bugs are reported but don't
> have to be treated as security.

You can treat config options like this, yes, but we can't for listing of
vulnerabilities.

> > > There was even a recent on-disk format change (mkfs required), this is
> > > not really for environments where security matters. Thanks.
> > 
> > It's a fix for a vulnerability, so I think it should stay assigned.  If
> > your system does not enable that config option, then there is nothing to
> > worry about, right?
> 
> It's not a vulnerability unless you define that very extensively. My
> systems don't enable that, distro kernels don't enable that, so there's
> nothing to worry about. Conclusiion is not to assign a CVE, right.
> Otherwise it only increases paperwork.

We don't control random company's need/want for paperwork, sorry.  We
are tasked with reporting all fixed vulnerabilities, and as always, we
do not know your use case (nor do we want to.)  So, should be an easy
"this does not pertain to us" check box to tick somewhere if what you
say above is true for your systems.

thanks,

greg k-h