[PATCH] btrfs: add missing ASSERT for system space_info in reserve_chunk_space

Jiasheng Jiang posted 1 patch 1 month ago
fs/btrfs/block-group.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] btrfs: add missing ASSERT for system space_info in reserve_chunk_space
Posted by Jiasheng Jiang 1 month ago
In reserve_chunk_space(), btrfs_find_space_info() is called to retrieve
the space_info for the SYSTEM block group. However, the returned pointer
is immediately dereferenced by spin_lock(&info->lock) without validation.

While the SYSTEM space_info is expected to be present during normal
operations, direct dereference without checking creates a risk of a
null pointer dereference. This deviates from the coding pattern seen
in peer functions like btrfs_chunk_alloc() where such returns are
validated with an ASSERT.

Add an ASSERT() to ensure the pointer is valid before access, improving
code robustness and consistency with the rest of the block-group logic.

Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
---
 fs/btrfs/block-group.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 08b14449fabe..f4229ee4f573 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -4313,6 +4313,7 @@ static void reserve_chunk_space(struct btrfs_trans_handle *trans,
 	lockdep_assert_held(&fs_info->chunk_mutex);
 
 	info = btrfs_find_space_info(fs_info, BTRFS_BLOCK_GROUP_SYSTEM);
+	ASSERT(info);
 	spin_lock(&info->lock);
 	left = info->total_bytes - btrfs_space_info_used(info, true);
 	spin_unlock(&info->lock);
-- 
2.25.1
Re: [PATCH] btrfs: add missing ASSERT for system space_info in reserve_chunk_space
Posted by Qu Wenruo 1 month ago

在 2026/1/9 06:15, Jiasheng Jiang 写道:
> In reserve_chunk_space(), btrfs_find_space_info() is called to retrieve
> the space_info for the SYSTEM block group. However, the returned pointer
> is immediately dereferenced by spin_lock(&info->lock) without validation.
> 
> While the SYSTEM space_info is expected to be present during normal
> operations, direct dereference without checking creates a risk of a
> null pointer dereference. This deviates from the coding pattern seen
> in peer functions like btrfs_chunk_alloc() where such returns are
> validated with an ASSERT.
> 
> Add an ASSERT() to ensure the pointer is valid before access, improving
> code robustness and consistency with the rest of the block-group logic.

If you know how system chunk works, you must understand without a system 
chunk a btrfs will never be mounted.

The ASSERT() looks unnecessary to me.

Thanks,
Qu

> 
> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
> ---
>   fs/btrfs/block-group.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 08b14449fabe..f4229ee4f573 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -4313,6 +4313,7 @@ static void reserve_chunk_space(struct btrfs_trans_handle *trans,
>   	lockdep_assert_held(&fs_info->chunk_mutex);
>   
>   	info = btrfs_find_space_info(fs_info, BTRFS_BLOCK_GROUP_SYSTEM);
> +	ASSERT(info);
>   	spin_lock(&info->lock);
>   	left = info->total_bytes - btrfs_space_info_used(info, true);
>   	spin_unlock(&info->lock);

Re: [PATCH] btrfs: add missing ASSERT for system space_info in reserve_chunk_space
Posted by David Sterba 4 weeks, 1 day ago
On Fri, Jan 09, 2026 at 10:02:00AM +1030, Qu Wenruo wrote:
> 
> 
> 在 2026/1/9 06:15, Jiasheng Jiang 写道:
> > In reserve_chunk_space(), btrfs_find_space_info() is called to retrieve
> > the space_info for the SYSTEM block group. However, the returned pointer
> > is immediately dereferenced by spin_lock(&info->lock) without validation.
> > 
> > While the SYSTEM space_info is expected to be present during normal
> > operations, direct dereference without checking creates a risk of a
> > null pointer dereference. This deviates from the coding pattern seen
> > in peer functions like btrfs_chunk_alloc() where such returns are
> > validated with an ASSERT.
> > 
> > Add an ASSERT() to ensure the pointer is valid before access, improving
> > code robustness and consistency with the rest of the block-group logic.
> 
> If you know how system chunk works, you must understand without a system 
> chunk a btrfs will never be mounted.
> 
> The ASSERT() looks unnecessary to me.

Agreed. I tried to look for justification or an assertion pattern we
might have yet incomplete but I don't see it. The system chunk assertion
is in some functions but not everywhere, was added when zoned code was
extended so this makes some sense. We also don't validate most of the
btrfs_find_space_info() calls and I don't think we should, besides the
initial setup that must lead to a real error.