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

[Jan Kara]

On Wed 08-07-26 05:24:08, 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, out_free
> 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>

I like the approach but it has an off-by-one error as well ;)

> @@ -635,8 +637,8 @@ int ext4_init_orphan_info(struct super_block *sb)
>  	iput(inode);
>  	return 0;
>  out_free:
> -	for (i--; i >= 0; i--)
> -		brelse(oi->of_binfo[i].ob_bh);
> +	while (loaded-- > 0)
			^^^ this should be >= 0.

								Honza

> +		brelse(oi->of_binfo[loaded].ob_bh);
>  	kvfree(oi->of_binfo);
>  out_put:
>  	iput(inode);
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR