* [PATCH v2] zonefs: fix page reference and BIO leak
@ 2020-12-10 1:38 Damien Le Moal
2020-12-10 2:53 ` Chaitanya Kulkarni
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Damien Le Moal @ 2020-12-10 1:38 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Johannes Thumshirn, Christoph Hellwig
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;
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] zonefs: fix page reference and BIO leak
2020-12-10 1:38 [PATCH v2] zonefs: fix page reference and BIO leak Damien Le Moal
@ 2020-12-10 2:53 ` Chaitanya Kulkarni
2020-12-10 5:28 ` Christoph Hellwig
2020-12-10 7:03 ` Johannes Thumshirn
2 siblings, 0 replies; 6+ messages in thread
From: Chaitanya Kulkarni @ 2020-12-10 2:53 UTC (permalink / raw)
To: Damien Le Moal, linux-fsdevel; +Cc: Johannes Thumshirn, Christoph Hellwig
On 12/9/20 17:41, 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
I think extra 'i' above needs to be removed at the time of applying the
patch.
> 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>
Looks good.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] zonefs: fix page reference and BIO leak
2020-12-10 1:38 [PATCH v2] zonefs: fix page reference and BIO leak Damien Le Moal
2020-12-10 2:53 ` Chaitanya Kulkarni
@ 2020-12-10 5:28 ` Christoph Hellwig
2020-12-10 7:03 ` Johannes Thumshirn
2 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2020-12-10 5:28 UTC (permalink / raw)
To: Damien Le Moal; +Cc: linux-fsdevel, Johannes Thumshirn, Christoph Hellwig
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] zonefs: fix page reference and BIO leak
2020-12-10 1:38 [PATCH v2] zonefs: fix page reference and BIO leak Damien Le Moal
2020-12-10 2:53 ` Chaitanya Kulkarni
2020-12-10 5:28 ` Christoph Hellwig
@ 2020-12-10 7:03 ` Johannes Thumshirn
2020-12-10 7:06 ` Christoph Hellwig
2 siblings, 1 reply; 6+ messages in thread
From: Johannes Thumshirn @ 2020-12-10 7:03 UTC (permalink / raw)
To: Damien Le Moal, linux-fsdevel; +Cc: Christoph Hellwig
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?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] zonefs: fix page reference and BIO leak
2020-12-10 7:03 ` Johannes Thumshirn
@ 2020-12-10 7:06 ` Christoph Hellwig
2020-12-10 7:11 ` Johannes Thumshirn
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-12-10 7:06 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: Damien Le Moal, linux-fsdevel, Christoph Hellwig
On Thu, Dec 10, 2020 at 07:03:08AM +0000, Johannes Thumshirn wrote:
> Aren't we loosing bio->bi_status = BLK_STS_IOERR in case bio_iov_iter_get_pages() fails now?
We do, but it does not matter because nothing actually looks at
->bi_status in this failure path.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] zonefs: fix page reference and BIO leak
2020-12-10 7:06 ` Christoph Hellwig
@ 2020-12-10 7:11 ` Johannes Thumshirn
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Thumshirn @ 2020-12-10 7:11 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Damien Le Moal, linux-fsdevel
On 10/12/2020 08:06, Christoph Hellwig wrote:
> On Thu, Dec 10, 2020 at 07:03:08AM +0000, Johannes Thumshirn wrote:
>> Aren't we loosing bio->bi_status = BLK_STS_IOERR in case bio_iov_iter_get_pages() fails now?
>
> We do, but it does not matter because nothing actually looks at
> ->bi_status in this failure path.
>
Right we never pass the bio to the block layer in the error case.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-10 7:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-10 1:38 [PATCH v2] zonefs: fix page reference and BIO leak Damien Le Moal
2020-12-10 2:53 ` Chaitanya Kulkarni
2020-12-10 5:28 ` Christoph Hellwig
2020-12-10 7:03 ` Johannes Thumshirn
2020-12-10 7:06 ` Christoph Hellwig
2020-12-10 7:11 ` Johannes Thumshirn
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.