Re: [PATCH v2] zonefs: fix page reference and BIO leak

[Johannes Thumshirn <Johannes.Thumshirn@wdc.com>]

On 10/12/2020 02:38, Damien Le Moal wrote:
> In zonefs_file_dio_append(), the pages obtained using
> bio_iov_iter_get_pages() are not released on completion of the
> REQ_OP_APPEND BIO, nor when bio_iov_iter_get_pages() fails.
> Furthermore, a call to bio_put() is missing when
> bio_iov_iter_get_pages() fails.
> 
> Fix these resource leaks by adding BIO resource release code (bio_put()i
> and bio_release_pages()) at the end of the function after the BIO
> execution and add a jump to this resource cleanup code in case of
> bio_iov_iter_get_pages() failure.
> 
> While at it, also fix the call to task_io_account_write() to be passed
> the correct BIO size instead of bio_iov_iter_get_pages() return value.
> 
> Reported-by: Christoph Hellwig <hch@lst.de>
> Fixes: 02ef12a663c7 ("zonefs: use REQ_OP_ZONE_APPEND for sync DIO")
> Cc: stable@vger.kernel.org
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  fs/zonefs/super.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
> index ff5930be096c..bec47f2d074b 100644
> --- a/fs/zonefs/super.c
> +++ b/fs/zonefs/super.c
> @@ -691,21 +691,23 @@ static ssize_t zonefs_file_dio_append(struct kiocb *iocb, struct iov_iter *from)
>  		bio->bi_opf |= REQ_FUA;
>  
>  	ret = bio_iov_iter_get_pages(bio, from);
> -	if (unlikely(ret)) {
> -		bio_io_error(bio);
> -		return ret;
> -	}
> +	if (unlikely(ret))
> +		goto out_release;
> +
>  	size = bio->bi_iter.bi_size;
> -	task_io_account_write(ret);
> +	task_io_account_write(size);
>  
>  	if (iocb->ki_flags & IOCB_HIPRI)
>  		bio_set_polled(bio, iocb);
>  
>  	ret = submit_bio_wait(bio);
>  
> +	zonefs_file_write_dio_end_io(iocb, size, ret, 0);
> +
> +out_release:
> +	bio_release_pages(bio, false);
>  	bio_put(bio);
>  
> -	zonefs_file_write_dio_end_io(iocb, size, ret, 0);
>  	if (ret >= 0) {
>  		iocb->ki_pos += size;
>  		return size;
> 

Aren't we loosing bio->bi_status = BLK_STS_IOERR in case bio_iov_iter_get_pages() fails now?