linux-kernel - Re: [PATCH] block: copy bi_vcnt in __bio_clone

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Fri, 10 Oct 2014 00:13:44 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Jeff Mahoney <jeffm@...e.com>
Cc:	Jeff Moyer <jmoyer@...hat.com>, Jens Axboe <axboe@...nel.dk>,
	Linux Kernel Maling List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] block: copy bi_vcnt in __bio_clone_fast

On Thu, Oct 9, 2014 at 11:25 PM, Ming Lei <ming.lei@...onical.com> wrote:
> On Thu, Oct 9, 2014 at 10:26 PM, Jeff Mahoney <jeffm@...e.com> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On 10/9/14, 9:53 AM, Jeff Moyer wrote:
>>> Jeff Mahoney <jeffm@...e.com> writes:
>>>
>>>> Commit 05f1dd53152173 (block: add queue flag for disabling SG
>>>> merging) uses bi_vcnt to assign bio->bi_phys_segments if sg
>>>> merging is disabled. When using device mapper on top of a blk-mq
>>>> device (virtio_blk in my test), we'd end up overflowing the
>>>> scatterlist in __blk_bios_map_sg.
>>>>
>>>> __bio_clone_fast copies bi_iter and bi_io_vec but not bi_vcnt,
>>>> so blk_recount_segments would report bi_phys_segments as 0.
>>>> Since rq->nr_phys_segments is 0 as well, the checks to ensure
>>>> that we don't exceed the queue's segment limit end up allowing
>>>> more bios (and segments) to attach the a request until we finally
>>>> map it. That also means we pass the BUG_ON at the beginning of
>>>> virtio_queue_rq, ultimately causing memory corruption and a
>>>> crash.
>>>>
>>>> If we copy bi_vcnt in __bio_clone_fast, the bios and requests
>>>> properly report the number of segments and everything works as
>>>> expected.
>>>>
>>>> Originally reported at
>>>> http://bugzilla.opensuse.org/show_bug.cgi?id=888259
>>>
>>> Hi, Jeff,
>>>
>>> Did you manage to reproduce this problem with commit 0738854
>>> (blk-merge: fix blk_recount_segments) applied?  Or perhaps with
>>> commit 200612e (dm table: propagate QUEUE_FLAG_NO_SG_MERGE)?
>>
>> Yep. I was able to reproduce it with 3.17. I did try 0738854 when I
>> was still using 3.16 since it looked like a good candidate. Neither of
>> those patches affect the problem here. bio->bi_phys_segments never
>> gets a value set in the fast clone case and that translates to
>> req->nr_phys_segments never getting properly accumulated. That might
>> not be a problem except that the NO_SG_MERGE behavior bypasses the
>> iteration that would come up with the correct value. In either case,
>
> This patch may get incorrect rq->nr_phys_segments because
> bio cloning is often used in case of I/O splitting, so could you
> test if the attached patch fixes your problem?

Please ignore last patch and test the attached patch since we
still should use no sg merge to recalculate req's segments for
cloned bio.

Thanks,

View attachment "0001-blk-merge-don-t-compute-bi_phys_segments-from-bi_vcn.patch" of type "text/x-patch" (1275 bytes)