linux-kernel - RE: [PATCH] block: return the correct bvec when checking for gaps

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 4 Jun 2021 08:58:17 +0000
From:   Long Li <longli@...rosoft.com>
To:     Ming Lei <ming.lei@...hat.com>
CC:     "longli@...uxonhyperv.com" <longli@...uxonhyperv.com>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        Jens Axboe <axboe@...nel.dk>,
        Johannes Thumshirn <johannes.thumshirn@....com>,
        Pavel Begunkov <asml.silence@...il.com>,
        Tejun Heo <tj@...nel.org>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Jeffle Xu <jefflexu@...ux.alibaba.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH] block: return the correct bvec when checking for gaps

> Subject: Re: [PATCH] block: return the correct bvec when checking for gaps
> 
> On Fri, Jun 04, 2021 at 06:38:45AM +0000, Long Li wrote:
> > > Subject: Re: [PATCH] block: return the correct bvec when checking
> > > for gaps
> > >
> > > Hello Long,
> > >
> > > On Thu, Jun 03, 2021 at 03:34:31PM -0700, longli@...uxonhyperv.com
> wrote:
> > > > From: Long Li <longli@...rosoft.com>
> > > >
> > > > After commit 07173c3ec276 ("block: enable multipage bvecs"), a
> > > > bvec can have multiple pages. But bio_will_gap() still assumes one
> > > > page bvec while checking for merging. This causes data corruption
> > > > on drivers relying on the correct merging on virt_boundary_mask.
> > >
> > > Can you explain the data corruption a bit?
> > >
> > > IMO, either single page bvec or multipage bvec should be fine,
> > > because
> > > bio_will_gap() just checks if the last bvec of prev bio and the 1st
> > > bvec of next bio can be merged.
> >
> > Hi Ming,
> >
> > When bio_will_gap() calls into biovec_phys_mergeable (),
> seg_boundary_mask (queue_segment_boundary()) is used to test if the two
> bio_vecs can be merged. This test can succeed if only the 1st page in bvec is
> used, but at the same time it can fail if all the pages in bvec are used. In other
> words, if the pages in bvec go across the seg_boundary_mask, the test can
> potentially succeed if only the 1st page is tested, but can fail if all the pages
> are tested.
> >
> > Later, when SCSI builds the SG list from BIOs (that calls into
> __blk_bios_map_sg), __blk_segment_map_sg_merge() calls
> biovec_phys_mergeable() doing the same test . This time it may fail if the
> pages in bvec go across the seg_boundary_mask (but tested okay in
> bio_will_gap() earlier, so those two BIOs were merged). If
> __blk_segment_map_sg_merge() fails, we end up with a broken SG list for
> drivers assuming the SG list not having offsets in intermediate pages.
> >
> 
> OK, the reason is that both bio_will_gap() and
> __blk_segment_map_sg_merge() have to use same approach to check if
> two bvecs from two bios can be mergeable.
> 
> Now __blk_segment_map_sg_merge() won't merge the 1st bvec of next bio
> into previous bio if the 1st bvec of next bio crosses segment boundary, so
> bio_will_gap() has to take same way to check if the two bvecs can be merged.
> 
> Please add the segment boundary and map SG list story in commit log, then
> the patch looks fine.

Sure, I will send v2.

Long


> 
> 
> Thanks,
> Ming