Re: [PATCH] btrfs: add a sanity check for btrfs root in btrfs_next_old_leaf()

[David Sterba <dsterba@suse.cz>]

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.