linux-kernel - Re: [PATCH] btrfs: add a sanity check for btrfs root in btrfs_next_old

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

Message-ID: <20250224094511.GK5777@twin.jikos.cz>
Date: Mon, 24 Feb 2025 10:45:11 +0100
From: David Sterba <dsterba@...e.cz>
To: Qu Wenruo <quwenruo.btrfs@....com>
Cc: Ma Ke <make24@...as.ac.cn>, clm@...com, josef@...icpanda.com,
	dsterba@...e.com, fdmanana@...e.com, linux-btrfs@...r.kernel.org,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] btrfs: add a sanity check for btrfs root in
 btrfs_next_old_leaf()

On Mon, Feb 24, 2025 at 06:48:19PM +1030, Qu Wenruo wrote:
> 在 2025/2/24 18:29, Ma Ke 写道:
> > btrfs_next_old_leaf() doesn't check if the target root is NULL or not,
> > resulting the null-ptr-deref. Add sanity check for btrfs root before
> > using it in btrfs_next_old_leaf().
> 
> Please provide a real world call trace when this is triggered.
> 
> There is a prerequisite, the extent tree can only be NULL if
> rescue=ibadroots is provided and the extent root is corrupted.
> 
> And "rescue=" mount options can only be specified for a fully read-only
> fs (no internal log replay or any other thing to write even a bit into
> the fs).
> 
> Previously read-only scrub can still be triggered on such fs, but
> 6aecd91a5c5b ("btrfs: avoid NULL pointer dereference if no valid extent
> tree") fixed that.
> 
> And if you hit such a case in real world, please provide the call trace
> so that we know we're not missing some critical situations that extent
> tree is accessed for read-only operations.

Agreed, this needs a real way to trigger it. Some pointers do not have
to be checked for NULL because it's guaranteed by some of the caller
higher up in the call chain.

Before we added the rescue options, the invariants were that the extent,
checksum, fs_tree etc always exist when passed as pointers. The example
fix 6aecd91a5c5b show it could be possible under some circumstances. So
if a fix is needed btrfs_next_old_leaf() we also need to see how it
could happen.