Re: [PATCH] btrfs: use the btrfs_block_group_end() helper everywhere

[Dan Carpenter <dan.carpenter@linaro.org>]

On Mon, Jan 19, 2026 at 10:27:56AM +0300, Dan Carpenter wrote:
> On Mon, Jan 19, 2026 at 06:49:34AM +0000, Johannes Thumshirn wrote:
> > On 1/19/26 7:40 AM, Dan Carpenter wrote:
> > > d4452bc526c431 Chris Mason     2014-05-19  1202  	u64 start, extent_start, extent_end, len;
> > > 7fc5a6968403c7 Filipe Manana   2026-01-16 @1203  	const u64 block_group_end = btrfs_block_group_end(block_group);
> > >                                                                                                            ^^^^^^^^^^^^
> > > Dereferenced.
> > >
> > > d4452bc526c431 Chris Mason     2014-05-19  1204  	struct extent_io_tree *unpin = NULL;
> > > d4452bc526c431 Chris Mason     2014-05-19  1205  	int ret;
> > > 43be21462d8c26 Josef Bacik     2011-04-01  1206
> > > 5349d6c3ffead2 Miao Xie        2014-06-19 @1207  	if (!block_group)
> > >                                                               ^^^^^^^^^^^
> > > Too late.
> > 
> > I _think_ this check is bogus.
> > 
> > On one hand  write_pinned_extent_entries() gets called by 
> > __btrfs_write_out_cache(), which has the following comment at the top:
> > 
> > /*
> >   * Write out cached info to an inode.
> >   *
> >   * @inode:       freespace inode we are writing out
> >   * @ctl:         free space cache we are going to write out
> >   * @block_group: block_group for this cache if it belongs to a block_group
> > 
> > but then __btrfs_write_out_cache() is only called by 
> > btrfs_write_out_cache() which looks like this:
> > 
> >      ret = __btrfs_write_out_cache(inode, ctl, block_group,
> >                        &block_group->io_ctl, trans);
> > 
> > so iff block_group really is NULL, we'd have a NULL pointer dereference 
> > when accessing block_group::io_ctl.
> > 
> > 
> > Same for all the if (block_group) constructs in __btrfs_write_out_cache().
> > 
> 
> The zero day bot can't use cross function analysis because it doesn't
> scale to the number of trees which the bot tests...  Otherwise, sure,
> Smatch would silence this warning.
> 
> If we remove the NULL check here, should we also remove the NULL checking
> in __btrfs_write_out_cache()?  Otherwise there is still a potential that

s/__btrfs_write_out_cache/write_cache_extent_entries/

Sorry.

Anyway, you were too quick for me and have already done this.

regards,
dan carpenter

> something will complain about inconsistent NULL checking.
> 
> regards,
> dan carpenter
>