* [PATCH 0/2] block: fix page leak by merging to same page
@ 2019-06-08 16:48 Ming Lei
2019-06-08 16:48 ` [PATCH 1/2] block: introduce 'enum bvec_merge_flags' for __bio_try_merge_page Ming Lei
2019-06-08 16:48 ` [PATCH 2/2] block: fix page leak in case of merging to same page Ming Lei
0 siblings, 2 replies; 4+ messages in thread
From: Ming Lei @ 2019-06-08 16:48 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Ming Lei, David Gibson, Darrick J. Wong, linux-xfs,
Alexander Viro, Christoph Hellwig
Hi,
'pages' retrived by __bio_iov_iter_get_pages() may point to same page,
and finally they can be merged to same page too in bio_add_page(),
then page leak can be caused.
Fixes this issue by dropping the extra page ref.
Ming Lei (2):
block: introduce 'enum bvec_merge_flags' for __bio_try_merge_page
block: fix page leak in case of merging to same page
block/bio.c | 34 ++++++++++++++++++++++++----------
fs/iomap.c | 3 ++-
fs/xfs/xfs_aops.c | 3 ++-
include/linux/bio.h | 9 ++++++++-
4 files changed, 36 insertions(+), 13 deletions(-)
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] block: introduce 'enum bvec_merge_flags' for __bio_try_merge_page
2019-06-08 16:48 [PATCH 0/2] block: fix page leak by merging to same page Ming Lei
@ 2019-06-08 16:48 ` Ming Lei
2019-06-08 16:48 ` [PATCH 2/2] block: fix page leak in case of merging to same page Ming Lei
1 sibling, 0 replies; 4+ messages in thread
From: Ming Lei @ 2019-06-08 16:48 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Ming Lei, Darrick J. Wong, linux-xfs,
Alexander Viro, Christoph Hellwig, David Gibson
Introduce 'enum bvec_merge_flags' and pass it to __bio_try_merge_page,
we have to deal with several cases related with page reference when
merging same page to bio(bvec), such as:
1) only merge to same page without putting reference of the same page,
such as iomap & xfs
2) merge to same page and put reference of the same page, such as
__bio_iov_iter_get_pages()
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/bio.c | 20 ++++++++++++--------
fs/iomap.c | 3 ++-
fs/xfs/xfs_aops.c | 3 ++-
include/linux/bio.h | 8 +++++++-
4 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 683cbb40f051..39e3b931dc3b 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -636,7 +636,7 @@ EXPORT_SYMBOL(bio_clone_fast);
static inline bool page_is_mergeable(const struct bio_vec *bv,
struct page *page, unsigned int len, unsigned int off,
- bool same_page)
+ enum bvec_merge_flags flags)
{
phys_addr_t vec_end_addr = page_to_phys(bv->bv_page) +
bv->bv_offset + bv->bv_len - 1;
@@ -648,13 +648,14 @@ static inline bool page_is_mergeable(const struct bio_vec *bv,
return false;
if ((vec_end_addr & PAGE_MASK) != page_addr) {
- if (same_page)
+ if (flags & BVEC_MERGE_TO_SAME_PAGE)
return false;
if (pfn_to_page(PFN_DOWN(vec_end_addr)) + 1 != page)
return false;
}
- WARN_ON_ONCE(same_page && (len + off) > PAGE_SIZE);
+ WARN_ON_ONCE((flags & BVEC_MERGE_TO_SAME_PAGE) &&
+ (len + off) > PAGE_SIZE);
return true;
}
@@ -729,8 +730,9 @@ static int __bio_add_pc_page(struct request_queue *q, struct bio *bio,
if (bvec_gap_to_prev(q, bvec, offset))
return 0;
- if (page_is_mergeable(bvec, page, len, offset, false) &&
- can_add_page_to_seg(q, bvec, page, len, offset)) {
+ if (page_is_mergeable(bvec, page, len, offset,
+ BVEC_MERGE_DEFAULT) && can_add_page_to_seg(q, bvec,
+ page, len, offset)) {
bvec->bv_len += len;
goto done;
}
@@ -779,7 +781,8 @@ EXPORT_SYMBOL(bio_add_pc_page);
* Return %true on success or %false on failure.
*/
bool __bio_try_merge_page(struct bio *bio, struct page *page,
- unsigned int len, unsigned int off, bool same_page)
+ unsigned int len, unsigned int off,
+ enum bvec_merge_flags flags)
{
if (WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED)))
return false;
@@ -787,7 +790,7 @@ bool __bio_try_merge_page(struct bio *bio, struct page *page,
if (bio->bi_vcnt > 0) {
struct bio_vec *bv = &bio->bi_io_vec[bio->bi_vcnt - 1];
- if (page_is_mergeable(bv, page, len, off, same_page)) {
+ if (page_is_mergeable(bv, page, len, off, flags)) {
bv->bv_len += len;
bio->bi_iter.bi_size += len;
return true;
@@ -837,7 +840,8 @@ EXPORT_SYMBOL_GPL(__bio_add_page);
int bio_add_page(struct bio *bio, struct page *page,
unsigned int len, unsigned int offset)
{
- if (!__bio_try_merge_page(bio, page, len, offset, false)) {
+ if (!__bio_try_merge_page(bio, page, len, offset,
+ BVEC_MERGE_DEFAULT)) {
if (bio_full(bio))
return 0;
__bio_add_page(bio, page, len, offset);
diff --git a/fs/iomap.c b/fs/iomap.c
index 23ef63fd1669..e04652bbf92a 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -316,7 +316,8 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
*/
sector = iomap_sector(iomap, pos);
if (ctx->bio && bio_end_sector(ctx->bio) == sector) {
- if (__bio_try_merge_page(ctx->bio, page, plen, poff, true))
+ if (__bio_try_merge_page(ctx->bio, page, plen, poff,
+ BVEC_MERGE_TO_SAME_PAGE))
goto done;
is_contig = true;
}
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index a6f0f4761a37..7e7385bc3b9e 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -774,7 +774,8 @@ xfs_add_to_ioend(
wpc->imap.br_state, offset, bdev, sector);
}
- if (!__bio_try_merge_page(wpc->ioend->io_bio, page, len, poff, true)) {
+ if (!__bio_try_merge_page(wpc->ioend->io_bio, page, len, poff,
+ BVEC_MERGE_TO_SAME_PAGE)) {
if (iop)
atomic_inc(&iop->write_count);
if (bio_full(wpc->ioend->io_bio))
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 0f23b5682640..48a95bca1703 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -419,11 +419,17 @@ extern void bio_uninit(struct bio *);
extern void bio_reset(struct bio *);
void bio_chain(struct bio *, struct bio *);
+enum bvec_merge_flags {
+ BVEC_MERGE_DEFAULT,
+ BVEC_MERGE_TO_SAME_PAGE = BIT(0),
+};
+
extern int bio_add_page(struct bio *, struct page *, unsigned int,unsigned int);
extern int bio_add_pc_page(struct request_queue *, struct bio *, struct page *,
unsigned int, unsigned int);
bool __bio_try_merge_page(struct bio *bio, struct page *page,
- unsigned int len, unsigned int off, bool same_page);
+ unsigned int len, unsigned int off,
+ enum bvec_merge_flags flags);
void __bio_add_page(struct bio *bio, struct page *page,
unsigned int len, unsigned int off);
int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter);
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] block: fix page leak in case of merging to same page
2019-06-08 16:48 [PATCH 0/2] block: fix page leak by merging to same page Ming Lei
2019-06-08 16:48 ` [PATCH 1/2] block: introduce 'enum bvec_merge_flags' for __bio_try_merge_page Ming Lei
@ 2019-06-08 16:48 ` Ming Lei
2019-06-10 3:40 ` Ming Lei
1 sibling, 1 reply; 4+ messages in thread
From: Ming Lei @ 2019-06-08 16:48 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Ming Lei, David Gibson, Darrick J. Wong, linux-xfs,
Alexander Viro, Christoph Hellwig
Different iovec may use one same page, then 'pages' array filled
by iov_iter_get_pages() may get reference of the same page several
times. If some page in 'pages' can be merged to same page in one bvec
by bio_add_page(), bio_release_pages() only drops the page's reference
once.
This way causes page leak reported by David Gibson.
This issue can be triggered since 576ed913 ("block: use bio_add_page in
bio_iov_iter_get_pages").
Fixes the issue by put the page's ref if it is merged to same page.
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Link: https://lkml.org/lkml/2019/4/23/64
Fixes: 576ed913 ("block: use bio_add_page in bio_iov_iter_get_pages")
Reported-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/bio.c | 14 ++++++++++++--
include/linux/bio.h | 1 +
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 39e3b931dc3b..5f7b46360c36 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -657,6 +657,10 @@ static inline bool page_is_mergeable(const struct bio_vec *bv,
WARN_ON_ONCE((flags & BVEC_MERGE_TO_SAME_PAGE) &&
(len + off) > PAGE_SIZE);
+ if ((flags & BVEC_MERGE_PUT_SAME_PAGE) &&
+ (flags & BVEC_MERGE_TO_SAME_PAGE))
+ put_page(page);
+
return true;
}
@@ -924,8 +928,14 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
struct page *page = pages[i];
len = min_t(size_t, PAGE_SIZE - offset, left);
- if (WARN_ON_ONCE(bio_add_page(bio, page, len, offset) != len))
- return -EINVAL;
+
+ if (!__bio_try_merge_page(bio, page, len, offset,
+ BVEC_MERGE_PUT_SAME_PAGE |
+ BVEC_MERGE_TO_SAME_PAGE)) {
+ if (WARN_ON_ONCE(bio_add_page(bio, page, len, offset)
+ != len))
+ return -EINVAL;
+ }
offset = 0;
}
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 48a95bca1703..dec6cf683d8e 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -422,6 +422,7 @@ void bio_chain(struct bio *, struct bio *);
enum bvec_merge_flags {
BVEC_MERGE_DEFAULT,
BVEC_MERGE_TO_SAME_PAGE = BIT(0),
+ BVEC_MERGE_PUT_SAME_PAGE = BIT(1),
};
extern int bio_add_page(struct bio *, struct page *, unsigned int,unsigned int);
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] block: fix page leak in case of merging to same page
2019-06-08 16:48 ` [PATCH 2/2] block: fix page leak in case of merging to same page Ming Lei
@ 2019-06-10 3:40 ` Ming Lei
0 siblings, 0 replies; 4+ messages in thread
From: Ming Lei @ 2019-06-10 3:40 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-block, David Gibson, Darrick J. Wong,
open list:XFS FILESYSTEM, Alexander Viro, Christoph Hellwig
On Sun, Jun 9, 2019 at 12:50 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Different iovec may use one same page, then 'pages' array filled
> by iov_iter_get_pages() may get reference of the same page several
> times. If some page in 'pages' can be merged to same page in one bvec
> by bio_add_page(), bio_release_pages() only drops the page's reference
> once.
>
> This way causes page leak reported by David Gibson.
>
> This issue can be triggered since 576ed913 ("block: use bio_add_page in
> bio_iov_iter_get_pages").
>
> Fixes the issue by put the page's ref if it is merged to same page.
>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: "Darrick J. Wong" <darrick.wong@oracle.com>
> Cc: linux-xfs@vger.kernel.org
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Christoph Hellwig <hch@infradead.org>
> Link: https://lkml.org/lkml/2019/4/23/64
> Fixes: 576ed913 ("block: use bio_add_page in bio_iov_iter_get_pages")
> Reported-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> block/bio.c | 14 ++++++++++++--
> include/linux/bio.h | 1 +
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 39e3b931dc3b..5f7b46360c36 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -657,6 +657,10 @@ static inline bool page_is_mergeable(const struct bio_vec *bv,
> WARN_ON_ONCE((flags & BVEC_MERGE_TO_SAME_PAGE) &&
> (len + off) > PAGE_SIZE);
>
> + if ((flags & BVEC_MERGE_PUT_SAME_PAGE) &&
> + (flags & BVEC_MERGE_TO_SAME_PAGE))
> + put_page(page);
> +
> return true;
> }
>
> @@ -924,8 +928,14 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
> struct page *page = pages[i];
>
> len = min_t(size_t, PAGE_SIZE - offset, left);
> - if (WARN_ON_ONCE(bio_add_page(bio, page, len, offset) != len))
> - return -EINVAL;
> +
> + if (!__bio_try_merge_page(bio, page, len, offset,
> + BVEC_MERGE_PUT_SAME_PAGE |
> + BVEC_MERGE_TO_SAME_PAGE)) {
oops, it is wrong to pass BVEC_MERGE_TO_SAME_PAGE here, which
should disable multi-page merge.
Please ignore this patchset, and will fix it in V2.
Thanks,
Ming Lei
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-10 3:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-08 16:48 [PATCH 0/2] block: fix page leak by merging to same page Ming Lei
2019-06-08 16:48 ` [PATCH 1/2] block: introduce 'enum bvec_merge_flags' for __bio_try_merge_page Ming Lei
2019-06-08 16:48 ` [PATCH 2/2] block: fix page leak in case of merging to same page Ming Lei
2019-06-10 3:40 ` Ming Lei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).