lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 21 Aug 2020 14:23:55 +0530 From: Ritesh Harjani <riteshh@...ux.ibm.com> To: Christoph Hellwig <hch@...radead.org>, Anju T Sudhakar <anju@...ux.vnet.ibm.com>, linux-block@...r.kernel.org Cc: darrick.wong@...cle.com, linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, willy@...radead.org Subject: Re: [PATCH] iomap: Fix the write_count in iomap_add_to_ioend(). On 8/21/20 11:37 AM, Christoph Hellwig wrote: > On Wed, Aug 19, 2020 at 03:58:41PM +0530, Anju T Sudhakar wrote: >> From: Ritesh Harjani <riteshh@...ux.ibm.com> >> >> __bio_try_merge_page() may return same_page = 1 and merged = 0. >> This could happen when bio->bi_iter.bi_size + len > UINT_MAX. >> Handle this case in iomap_add_to_ioend() by incrementing write_count. >> This scenario mostly happens where we have too much dirty data accumulated. >> >> w/o the patch we hit below kernel warning, > > I think this is better fixed in the block layer rather than working > around the problem in the callers. Something like this: > > diff --git a/block/bio.c b/block/bio.c > index c63ba04bd62967..ef321cd1072e4e 100644 > --- a/block/bio.c > +++ b/block/bio.c > @@ -879,8 +879,10 @@ bool __bio_try_merge_page(struct bio *bio, struct page *page, > struct bio_vec *bv = &bio->bi_io_vec[bio->bi_vcnt - 1]; > > if (page_is_mergeable(bv, page, len, off, same_page)) { > - if (bio->bi_iter.bi_size > UINT_MAX - len) > + if (bio->bi_iter.bi_size > UINT_MAX - len) { > + *same_page = false; > return false; > + } > bv->bv_len += len; > bio->bi_iter.bi_size += len; > return true; > Ya, we had think of that. But what we then thought was, maybe the API does return the right thing. Meaning, what API says is, same_page is true, but the page couldn't be merged hence it returned ret = false. With that thought, we fixed this in the caller. But agree with you that with ret = false, there is no meaning of same_page being true. Ok, so let linux-block comment on whether above also looks good. If yes, I can spin out an official patch with all details. -ritesh
Powered by blists - more mailing lists