Re: [PATCH v2] ext4: fix buffer_head leak in ext4_init_orphan_info

[Jan Kara]

On Thu 09-07-26 14:41:51, Guanghui Yang wrote:
> ext4_init_orphan_info() reads orphan file blocks with ext4_bread()
> and stores the returned buffer_head in oi->of_binfo[i].ob_bh.
> 
> If ext4_bread() succeeds but the orphan block magic or checksum
> validation fails, the function jumps to out_free. However, the old
> out_free loop starts releasing buffers from i - 1, so the current
> buffer_head at index i is skipped.
> 
> This leaks the buffer_head reference obtained by ext4_bread() on the
> bad magic and bad checksum error paths.
> 
> Fix this by tracking the number of successfully read buffer_heads and
> releasing exactly those buffer_heads on the error path.
> 
> Fixes: 02f310fcf47f ("ext4: Speedup ext4 orphan inode handling")
> Signed-off-by: Guanghui Yang <3497809730@xxxxxx>

Right, your previous code was actually correct as well with the
post-decrement. I just got confused, sorry for that.  I think the most
idiomatic way to write the cleanup loop is like:

> -	for (i--; i >= 0; i--)
> -		brelse(oi->of_binfo[i].ob_bh);
> +	while (loaded > 0) {
> +		loaded--;
> +		brelse(oi->of_binfo[loaded].ob_bh);
> +	}

	while (--loaded >= 0)
		brelse(oi->of_binfo[loaded].ob_bh);

But regardless feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

>  	kvfree(oi->of_binfo);
>  out_put:
>  	iput(inode);
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR